diff --git a/modules/certmanager/go.mod b/modules/certmanager/go.mod index d5cabce3..d0b8b636 100644 --- a/modules/certmanager/go.mod +++ b/modules/certmanager/go.mod @@ -90,6 +90,8 @@ replace github.com/openstack-k8s-operators/lib-common/modules/common => ../commo replace github.com/openstack-k8s-operators/lib-common/modules/test => ../test +replace github.com/openstack-k8s-operators/lib-common/modules/openstack => ../openstack + // mschuppert: map to latest commit from release-4.13 tag // must consistent within modules and service operators replace github.com/openshift/api => github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 diff --git a/modules/certmanager/go.sum b/modules/certmanager/go.sum index 2fdeea13..b337a108 100644 --- a/modules/certmanager/go.sum +++ b/modules/certmanager/go.sum @@ -240,8 +240,6 @@ github.com/openstack-k8s-operators/infra-operator/apis v0.1.1-0.20230914145253-1 github.com/openstack-k8s-operators/infra-operator/apis v0.1.1-0.20230914145253-116f307c7875/go.mod h1:NgrvT3CKMu6fE8Nt1H79qHx11L3I7Bb2eItniM7c9ow= github.com/openstack-k8s-operators/keystone-operator/api v0.1.1-0.20230914163026-da9aa9de960a h1:MFYwi2Xk9r3OMPToCSbvqYVNrm7P+aFzGDN0eVNpgu8= github.com/openstack-k8s-operators/keystone-operator/api v0.1.1-0.20230914163026-da9aa9de960a/go.mod h1:nxrbUOIGMJ1h2pNlawhURdt/XJ95dW2wZGedmOVo2aw= -github.com/openstack-k8s-operators/lib-common/modules/openstack v0.1.1-0.20230913075424-2680ce4b6ad2 h1:4L5DRfSnomBfyRwCfAzqQwk0+osnIbyJ1VvKBy+tzyY= -github.com/openstack-k8s-operators/lib-common/modules/openstack v0.1.1-0.20230913075424-2680ce4b6ad2/go.mod h1:NZ6weu5xOAkNPTqg1luC22DO7ZbyqiilRkvrFfhjFm0= github.com/openstack-k8s-operators/mariadb-operator/api v0.1.1-0.20230913081601-9e4fc8aadad5 h1:dQcSQuXfgzgOhc4v+zD0jE6WWhn6FHr5nALOjJBPxyI= github.com/openstack-k8s-operators/mariadb-operator/api v0.1.1-0.20230913081601-9e4fc8aadad5/go.mod h1:mJyhm/YiQZaYhLvOuLng/ITpwx8HvsYVht+VotS1Ed8= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= diff --git a/modules/common/endpoint/endpoint.go b/modules/common/endpoint/endpoint.go index 91c74da1..4f513346 100644 --- a/modules/common/endpoint/endpoint.go +++ b/modules/common/endpoint/endpoint.go @@ -18,6 +18,7 @@ package endpoint import ( "context" + "fmt" "net/url" "strings" "time" @@ -50,13 +51,21 @@ type Data struct { Port int32 // An optional path suffix to append to route hostname when forming Keystone endpoint URLs Path string + // protocol of the endpoint (http/https/none) + Protocol *service.Protocol // details for metallb service generation + // NOTE: (mschuppert) deprecated, can be removed when external endpoint creation moved to openstack-operator + // and ExposeEndpoints() can be removed MetalLB *MetalLBData // possible overrides for Route + // NOTE: (mschuppert) deprecated, can be removed when external endpoint creation moved to openstack-operator + // and ExposeEndpoints() can be removed RouteOverride *route.OverrideSpec } // MetalLBData - information specific to creating the MetalLB service +// NOTE: (mschuppert) deprecated, can be removed when external endpoint creation moved to openstack-operator +// and ExposeEndpoints() can be removed type MetalLBData struct { // Name of the metallb IpAddressPool IPAddressPool string @@ -73,12 +82,14 @@ type MetalLBData struct { } // ExposeEndpoints - creates services, routes and returns a map of created openstack endpoint +// NOTE: (mschuppert) deprecated, can be removed when external endpoint creation moved to openstack-operator +// and ExposeEndpoints() can be removed func ExposeEndpoints( ctx context.Context, h *helper.Helper, serviceName string, endpointSelector map[string]string, - endpoints map[Endpoint]Data, + endpoints map[service.Endpoint]Data, timeout time.Duration, ) (map[string]string, ctrl.Result, error) { endpointMap := make(map[string]string) @@ -95,6 +106,7 @@ func ExposeEndpoints( // Create metallb service if specified, otherwise create a route var hostname string + var port string if data.MetalLB != nil { var protocol corev1.Protocol if data.MetalLB.Protocol != nil { @@ -105,7 +117,7 @@ func ExposeEndpoints( } // Create the service - svc := service.NewService( + svc, err := service.NewService( service.MetalLBService(&service.MetalLBServiceDetails{ Name: endpointName, Namespace: h.GetBeforeObject().GetNamespace(), @@ -117,12 +129,15 @@ func ExposeEndpoints( Protocol: protocol, }, }), - exportLabels, timeout, + &service.OverrideSpec{}, ) + if err != nil { + return endpointMap, ctrl.Result{}, err + } annotations := map[string]string{ service.MetalLBAddressPoolAnnotation: data.MetalLB.IPAddressPool, - AnnotationHostnameKey: svc.GetServiceHostname(), // add annotation to register service name in dnsmasq + service.AnnotationHostnameKey: svc.GetServiceHostname(), // add annotation to register service name in dnsmasq } if len(data.MetalLB.LoadBalancerIPs) > 0 { annotations[service.MetalLBLoadBalancerIPs] = strings.Join(data.MetalLB.LoadBalancerIPs, ",") @@ -144,11 +159,11 @@ func ExposeEndpoints( } // create service - end - hostname = svc.GetServiceHostnamePort() + hostname, port = svc.GetServiceHostnamePort() } else { // Create the service - svc := service.NewService( + svc, err := service.NewService( service.GenericService(&service.GenericServiceDetails{ Name: endpointName, Namespace: h.GetBeforeObject().GetNamespace(), @@ -159,9 +174,13 @@ func ExposeEndpoints( Port: data.Port, Protocol: corev1.ProtocolTCP, }}), - exportLabels, 5, + &service.OverrideSpec{}, ) + if err != nil { + return endpointMap, ctrl.Result{}, err + } + ctrlResult, err := svc.CreateOrPatch(ctx, h) if err != nil { return endpointMap, ctrlResult, err @@ -170,10 +189,10 @@ func ExposeEndpoints( } // create service - end - hostname = svc.GetServiceHostnamePort() + hostname, port = svc.GetServiceHostnamePort() // Create the route if it is public endpoint - if endpointType == EndpointPublic { + if endpointType == service.EndpointPublic { // Create the route // TODO TLS route, err := route.NewRoute( @@ -184,7 +203,6 @@ func ExposeEndpoints( ServiceName: endpointName, TargetPortName: endpointName, }), - exportLabels, timeout, data.RouteOverride, ) @@ -216,7 +234,8 @@ func ExposeEndpoints( // Do not include data.Path in parsing check because %(project_id)s // is invalid without being encoded, but they should not be encoded in the actual endpoint - apiEndpoint, err := url.Parse(protocol + hostname) + endptURL := fmt.Sprintf("%s://%s:%s", protocol, hostname, port) + apiEndpoint, err := url.Parse(endptURL) if err != nil { return endpointMap, ctrl.Result{}, err } diff --git a/modules/common/route/route.go b/modules/common/route/route.go index c6ba5d12..e0b75734 100644 --- a/modules/common/route/route.go +++ b/modules/common/route/route.go @@ -37,7 +37,6 @@ import ( // NewRoute returns an initialized Route. func NewRoute( route *routev1.Route, - labels map[string]string, timeout time.Duration, override *OverrideSpec, ) (*Route, error) { @@ -89,6 +88,11 @@ func (r *Route) GetHostname() string { return r.hostname } +// GetRoute - returns the route object +func (r *Route) GetRoute() *routev1.Route { + return r.route +} + // GenericRoute func func GenericRoute(routeInfo *GenericRouteDetails) *routev1.Route { serviceRef := routev1.RouteTargetReference{ @@ -141,6 +145,18 @@ func (r *Route) CreateOrPatch( return err } + // Add the service CR to the ownerRef list of the route to prevent the route being deleted + // before the service is deleted. Otherwise this can result cleanup issues which require + // the endpoint to be reachable. + // If ALL objects in the list have been deleted, this object will be garbage collected. + // https://github.com/kubernetes/apimachinery/blob/15d95c0b2af3f4fcf46dce24105e5fbb9379af5a/pkg/apis/meta/v1/types.go#L240-L247 + for _, owner := range r.OwnerReferences { + err := controllerutil.SetOwnerReference(owner, route, h.GetScheme()) + if err != nil { + return err + } + } + return nil }) if err != nil { diff --git a/modules/common/route/types.go b/modules/common/route/types.go index 5f0b4568..d7f48d5c 100644 --- a/modules/common/route/types.go +++ b/modules/common/route/types.go @@ -22,16 +22,20 @@ import ( "time" routev1 "github.com/openshift/api/route/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // Route - +// +kubebuilder:object:generate:=false type Route struct { - route *routev1.Route - timeout time.Duration - hostname string + route *routev1.Route + timeout time.Duration + hostname string + OwnerReferences []metav1.Object } // GenericRouteDetails - +// +kubebuilder:object:generate:=false type GenericRouteDetails struct { Name string Namespace string diff --git a/modules/common/route/zz_generated.deepcopy.go b/modules/common/route/zz_generated.deepcopy.go index b60d3ce1..74f38c0b 100644 --- a/modules/common/route/zz_generated.deepcopy.go +++ b/modules/common/route/zz_generated.deepcopy.go @@ -54,28 +54,6 @@ func (in *EmbeddedLabelsAnnotations) DeepCopy() *EmbeddedLabelsAnnotations { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *GenericRouteDetails) DeepCopyInto(out *GenericRouteDetails) { - *out = *in - if in.Labels != nil { - in, out := &in.Labels, &out.Labels - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GenericRouteDetails. -func (in *GenericRouteDetails) DeepCopy() *GenericRouteDetails { - if in == nil { - return nil - } - out := new(GenericRouteDetails) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OverrideSpec) DeepCopyInto(out *OverrideSpec) { *out = *in @@ -101,26 +79,6 @@ func (in *OverrideSpec) DeepCopy() *OverrideSpec { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Route) DeepCopyInto(out *Route) { - *out = *in - if in.route != nil { - in, out := &in.route, &out.route - *out = new(v1.Route) - (*in).DeepCopyInto(*out) - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Route. -func (in *Route) DeepCopy() *Route { - if in == nil { - return nil - } - out := new(Route) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Spec) DeepCopyInto(out *Spec) { *out = *in diff --git a/modules/common/service/service.go b/modules/common/service/service.go index c17cc863..dbb54952 100644 --- a/modules/common/service/service.go +++ b/modules/common/service/service.go @@ -18,7 +18,10 @@ package service import ( "context" + "encoding/json" "fmt" + "net/url" + "strconv" "time" corev1 "k8s.io/api/core/v1" @@ -26,6 +29,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/strategicpatch" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -37,14 +41,51 @@ import ( // NewService returns an initialized Service. func NewService( service *corev1.Service, - labels map[string]string, timeout time.Duration, -) *Service { - return &Service{ + override *OverrideSpec, +) (*Service, error) { + svc := &Service{ service: service, serviceHostname: fmt.Sprintf("%s.%s.svc", service.Name, service.GetNamespace()), timeout: timeout, } + + // patch service with possible overrides of Labels, Annotations and Spec + if override != nil { + if override.EmbeddedLabelsAnnotations != nil { + if override.Labels != nil { + svc.service.Labels = util.MergeStringMaps(override.Labels, service.Labels) + } + if override.Annotations != nil { + svc.service.Annotations = util.MergeStringMaps(override.Annotations, service.Annotations) + } + } + if override.Spec != nil { + originalSpec, err := json.Marshal(service.Spec) + if err != nil { + return svc, fmt.Errorf("error marshalling Service Spec: %w", err) + } + + patch, err := json.Marshal(override.Spec) + if err != nil { + return svc, fmt.Errorf("error marshalling Service Spec override: %w", err) + } + + patchedJSON, err := strategicpatch.StrategicMergePatch(originalSpec, patch, corev1.ServiceSpec{}) + if err != nil { + return svc, fmt.Errorf("error patching Service Spec: %w", err) + } + + patchedSpec := corev1.ServiceSpec{} + err = json.Unmarshal(patchedJSON, &patchedSpec) + if err != nil { + return svc, fmt.Errorf("error unmarshalling patched Service Spec: %w", err) + } + svc.service.Spec = patchedSpec + } + } + + return svc, nil } // GetClusterIPs - returns the cluster IPs of the created service @@ -67,9 +108,37 @@ func (s *Service) GetServiceHostname() string { return s.serviceHostname } -// GetServiceHostnamePort - returns the service hostname with port -func (s *Service) GetServiceHostnamePort() string { - return fmt.Sprintf("%s:%d", s.GetServiceHostname(), GetServicesPortDetails(s.service, s.service.Name).Port) +// GetServiceHostnamePort - returns the service hostname with port if service port +// is not nil, otherwise returns GetServiceHostname() +func (s *Service) GetServiceHostnamePort() (string, string) { + servicePort := GetServicesPortDetails(s.service, s.service.Name) + if servicePort != nil { + return s.GetServiceHostname(), + strconv.FormatInt(int64(servicePort.Port), 10) + } + + return s.GetServiceHostname(), "" +} + +// GetLabels - returns labels of the service +func (s *Service) GetLabels() map[string]string { + return s.service.Labels +} + +// GetAnnotations - returns annotations of the service +func (s *Service) GetAnnotations() map[string]string { + return s.service.Annotations +} + +// GetSpec - returns the spec of the service +func (s *Service) GetSpec() *corev1.ServiceSpec { + spec := s.service.Spec + return &spec +} + +// GetServiceType - returns type of the service spec +func (s *Service) GetServiceType() corev1.ServiceType { + return s.service.Spec.Type } // AddAnnotation - Adds annotation and merges it with the current set @@ -77,8 +146,73 @@ func (s *Service) AddAnnotation(anno map[string]string) { s.service.Annotations = util.MergeStringMaps(s.service.Annotations, anno) } +// GetAPIEndpoint - returns the API endpoint URL for the service to register in keystone. +func (s *Service) GetAPIEndpoint(endpointURL *string, protocol *Protocol, path string) (string, error) { + var apiEndpoint *url.URL + var err error + if endpointURL != nil { + apiEndpoint, err = url.Parse(*endpointURL) + if err != nil { + return "", err + } + } else { + hostname, port := s.GetServiceHostnamePort() + + var endptURL string + if protocol != nil && + ((*protocol == ProtocolHTTP && port == "80") || + (*protocol == ProtocolHTTPS && port == "443")) { + endptURL = fmt.Sprintf("%s%s", EndptProtocol(protocol), hostname) + } else { + endptURL = fmt.Sprintf("%s%s:%s", EndptProtocol(protocol), hostname, port) + } + + // Do not include the path in parsing check because %(project_id)s + // is invalid without being encoded, but they should not be encoded in the actual endpoint + apiEndpoint, err = url.Parse(endptURL) + if err != nil { + return "", err + } + } + + return apiEndpoint.String() + path, nil +} + +// ToOverrideServiceSpec - convert corev1.ServiceSpec to OverrideServiceSpec +func (s *Service) ToOverrideServiceSpec() (*OverrideServiceSpec, error) { + overrideServiceSpec := &OverrideServiceSpec{} + + serviceSpec := s.GetSpec() + if serviceSpec != nil { + serviceSpecBytes, err := json.Marshal(serviceSpec) + if err != nil { + return nil, fmt.Errorf("error marshalling Service Spec: %w", err) + } + + err = json.Unmarshal(serviceSpecBytes, overrideServiceSpec) + if err != nil { + return nil, fmt.Errorf("error unmarshalling service OverrideSpec: %w", err) + } + } + + return overrideServiceSpec, nil +} + // GenericService func func GenericService(svcInfo *GenericServiceDetails) *corev1.Service { + ports := svcInfo.Ports + if len(svcInfo.Ports) == 0 { + ports = []corev1.ServicePort{ + { + Name: svcInfo.Port.Name, + Port: svcInfo.Port.Port, + // corev1.ProtocolTCP/ corev1.ProtocolUDP/ corev1.ProtocolSCTP + // - https://pkg.go.dev/k8s.io/api@v0.23.6/core/v1#Protocol + Protocol: svcInfo.Port.Protocol, + }, + } + + } return &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: svcInfo.Name, @@ -86,23 +220,31 @@ func GenericService(svcInfo *GenericServiceDetails) *corev1.Service { Labels: svcInfo.Labels, }, Spec: corev1.ServiceSpec{ - Selector: svcInfo.Selector, - Ports: []corev1.ServicePort{ - { - Name: svcInfo.Port.Name, - Port: svcInfo.Port.Port, - // corev1.ProtocolTCP/ corev1.ProtocolUDP/ corev1.ProtocolSCTP - // - https://pkg.go.dev/k8s.io/api@v0.23.6/core/v1#Protocol - Protocol: svcInfo.Port.Protocol, - }, - }, + Selector: svcInfo.Selector, + Ports: ports, ClusterIP: svcInfo.ClusterIP, + Type: corev1.ServiceTypeClusterIP, }, } } // MetalLBService func +// NOTE: (mschuppert) deprecated, can be removed when external endpoint creation moved to openstack-operator func MetalLBService(svcInfo *MetalLBServiceDetails) *corev1.Service { + ports := svcInfo.Ports + if len(svcInfo.Ports) == 0 { + ports = []corev1.ServicePort{ + { + Name: svcInfo.Port.Name, + Port: svcInfo.Port.Port, + // corev1.ProtocolTCP/ corev1.ProtocolUDP/ corev1.ProtocolSCTP + // - https://pkg.go.dev/k8s.io/api@v0.23.6/core/v1#Protocol + Protocol: svcInfo.Port.Protocol, + }, + } + + } + return &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: svcInfo.Name, @@ -112,16 +254,8 @@ func MetalLBService(svcInfo *MetalLBServiceDetails) *corev1.Service { }, Spec: corev1.ServiceSpec{ Selector: svcInfo.Selector, - Ports: []corev1.ServicePort{ - { - Name: svcInfo.Port.Name, - Port: svcInfo.Port.Port, - // corev1.ProtocolTCP/ corev1.ProtocolUDP/ corev1.ProtocolSCTP - // - https://pkg.go.dev/k8s.io/api@v0.23.6/core/v1#Protocol - Protocol: svcInfo.Port.Protocol, - }, - }, - Type: corev1.ServiceTypeLoadBalancer, + Ports: ports, + Type: corev1.ServiceTypeLoadBalancer, }, } } @@ -139,8 +273,8 @@ func (s *Service) CreateOrPatch( } op, err := controllerutil.CreateOrPatch(ctx, h.GetClient(), service, func() error { - service.Labels = util.MergeStringMaps(service.Labels, s.service.Labels) - service.Annotations = util.MergeStringMaps(service.Annotations, s.service.Annotations) + service.Labels = s.service.Labels + service.Annotations = s.service.Annotations service.Spec = s.service.Spec err := controllerutil.SetControllerReference(h.GetBeforeObject(), service, h.GetScheme()) @@ -280,3 +414,15 @@ func GetServicesPortDetails( return nil } + +// EndptProtocol returns the protocol for the endpoint if proto is nil http is considered +func EndptProtocol(proto *Protocol) string { + if proto == nil { + return string(ProtocolHTTP) + "://" + } + if *proto == ProtocolNone { + return "" + } + + return string(*proto) + "://" +} diff --git a/modules/common/service/service_test.go b/modules/common/service/service_test.go new file mode 100644 index 00000000..6e68c736 --- /dev/null +++ b/modules/common/service/service_test.go @@ -0,0 +1,426 @@ +/* +Copyright 2022 Red Hat + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// +kubebuilder:object:generate:=true + +package service + +import ( + "testing" + "time" + + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" +) + +var ( + svcClusterIP = corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "namespace", + Labels: map[string]string{ + "foo": "bar", + }, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{}, + Type: corev1.ServiceTypeClusterIP, + }, + } + svcLoadBalancer = corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "namespace", + Labels: map[string]string{ + "foo": "bar", + }}, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{}, + Type: corev1.ServiceTypeLoadBalancer, + }, + } + portHTTP = []corev1.ServicePort{ + { + Name: "foo", + Protocol: corev1.ProtocolTCP, + AppProtocol: nil, + Port: int32(80), + TargetPort: intstr.FromInt(0), + NodePort: 0, + }, + } + portHTTPS = []corev1.ServicePort{ + { + Name: "foo", + Protocol: corev1.ProtocolTCP, + AppProtocol: nil, + Port: int32(443), + TargetPort: intstr.FromInt(0), + NodePort: 0, + }, + } + portCustom = []corev1.ServicePort{ + { + Name: "foo", + Protocol: corev1.ProtocolTCP, + AppProtocol: nil, + Port: int32(8080), + TargetPort: intstr.FromInt(0), + NodePort: 0, + }, + } + timeout = time.Duration(5) * time.Second + override = OverrideSpec{ + Spec: &OverrideServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + }, + } + overrideServiceSpecClusterIP = OverrideServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + } + overrideServiceSpecLoadBalancer = OverrideServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + } +) + +func TestGenericService(t *testing.T) { + tests := []struct { + name string + service GenericServiceDetails + want corev1.Service + }{ + { + name: "Service with port, no labels, selector", + service: GenericServiceDetails{ + Name: "foo", + Namespace: "namespace", + Labels: map[string]string{}, + Selector: map[string]string{}, + Ports: []corev1.ServicePort{ + { + Name: "port", + Port: int32(80), + Protocol: corev1.ProtocolTCP, + }, + }, + }, + want: corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "namespace", + Labels: map[string]string{}, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "port", + Protocol: corev1.ProtocolTCP, + AppProtocol: nil, + Port: int32(80), + TargetPort: intstr.FromInt(0), + NodePort: 0, + }, + }, + Selector: map[string]string{}, + Type: corev1.ServiceTypeClusterIP, + }, + }, + }, + { + name: "Service with port, labels, selector", + service: GenericServiceDetails{ + Name: "foo", + Namespace: "namespace", + Labels: map[string]string{ + "foo": "bar", + }, + Selector: map[string]string{ + "foo": "bar", + }, + Ports: []corev1.ServicePort{ + { + Name: "port", + Port: int32(80), + Protocol: corev1.ProtocolTCP, + }, + }, + }, + want: corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "namespace", + Labels: map[string]string{ + "foo": "bar", + }, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "port", + Protocol: corev1.ProtocolTCP, + AppProtocol: nil, + Port: int32(80), + TargetPort: intstr.FromInt(0), + NodePort: 0, + }, + }, + Selector: map[string]string{ + "foo": "bar", + }, + Type: corev1.ServiceTypeClusterIP, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + service := GenericService(&tt.service) + + g.Expect(*service).To(Equal(tt.want)) + }) + } +} + +func getServiceWithPort(svc corev1.Service, ports []corev1.ServicePort) *corev1.Service { + svc.Spec.Ports = ports + + return &svc +} + +func TestNewService(t *testing.T) { + tests := []struct { + name string + service *corev1.Service + override OverrideSpec + want Service + wantPort string + wantOverrideServiceSpec OverrideServiceSpec + }{ + { + name: "HTTP ClusterIP service no override", + service: getServiceWithPort(svcClusterIP, portHTTP), + override: OverrideSpec{}, + want: Service{ + service: getServiceWithPort(svcClusterIP, portHTTP), + timeout: timeout, + serviceHostname: "foo.namespace.svc", + }, + wantPort: "80", + wantOverrideServiceSpec: overrideServiceSpecClusterIP, + }, + { + name: "HTTPS ClusterIP service no override", + service: getServiceWithPort(svcClusterIP, portHTTPS), + override: OverrideSpec{}, + want: Service{ + service: getServiceWithPort(svcClusterIP, portHTTPS), + timeout: timeout, + serviceHostname: "foo.namespace.svc", + }, + wantPort: "443", + wantOverrideServiceSpec: overrideServiceSpecClusterIP, + }, + { + name: "None ClusterIP service no override", + service: getServiceWithPort(svcClusterIP, portCustom), + override: OverrideSpec{}, + want: Service{ + service: getServiceWithPort(svcClusterIP, portCustom), + timeout: timeout, + serviceHostname: "foo.namespace.svc", + }, + wantPort: "8080", + wantOverrideServiceSpec: overrideServiceSpecClusterIP, + }, + { + name: "HTTP ClusterIP service override service Type to LoadBalancer", + service: getServiceWithPort(svcClusterIP, portHTTP), + override: override, + want: Service{ + service: getServiceWithPort(svcLoadBalancer, portHTTP), + timeout: timeout, + serviceHostname: "foo.namespace.svc", + }, + wantPort: "80", + wantOverrideServiceSpec: overrideServiceSpecLoadBalancer, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + service, err := NewService(tt.service, timeout, &tt.override) + g.Expect(err).ToNot(HaveOccurred()) + // timeout + g.Expect(service.timeout).To(Equal(timeout)) + // GetServiceType + g.Expect(service.GetServiceType()).To(Equal(tt.want.service.Spec.Type)) + // GetLabels + g.Expect(service.GetLabels()).To(Equal(map[string]string{ + "foo": "bar", + })) + // AddAnnotation + service.AddAnnotation(map[string]string{"foo": "bar"}) + // GetAnnotations + g.Expect(service.GetAnnotations()).To(Equal(map[string]string{"foo": "bar"})) + // GetServiceHostname + g.Expect(service.GetServiceHostname()).To(Equal(tt.want.serviceHostname)) + // GetServiceHostnamePort + hostname, port := service.GetServiceHostnamePort() + g.Expect(hostname).To(Equal(tt.want.serviceHostname)) + g.Expect(port).To(Equal(tt.wantPort)) + // GetSpec + g.Expect(*service.GetSpec()).To(Equal(tt.want.service.Spec)) + // ToOverrideServiceSpec + dd, err := service.ToOverrideServiceSpec() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(dd).ToNot(BeNil()) + g.Expect(*dd).To(Equal(tt.wantOverrideServiceSpec)) + }) + } +} + +func TestGetAPIEndpoint(t *testing.T) { + tests := []struct { + name string + service *corev1.Service + endpointURL *string + proto Protocol + port string + path string + want string + }{ + { + name: "HTTP ClusterIP service default port 80, no override", + service: getServiceWithPort(svcClusterIP, portHTTP), + endpointURL: nil, + proto: ProtocolHTTP, + path: "", + want: "http://foo.namespace.svc", + }, + { + name: "HTTP ClusterIP service non default 8080 port, no override", + service: getServiceWithPort(svcClusterIP, portCustom), + endpointURL: nil, + proto: ProtocolHTTP, + path: "/path", + want: "http://foo.namespace.svc:8080/path", + }, + { + name: "HTTPS ClusterIP service default 443 port, no override", + service: getServiceWithPort(svcClusterIP, portHTTPS), + endpointURL: nil, + proto: ProtocolHTTPS, + path: "/path", + want: "https://foo.namespace.svc/path", + }, + { + name: "HTTPS ClusterIP service non default 8080 port, no override", + service: getServiceWithPort(svcClusterIP, portCustom), + endpointURL: nil, + proto: ProtocolHTTPS, + path: "/path", + want: "https://foo.namespace.svc:8080/path", + }, + { + name: "None ClusterIP service port 80 no override", + service: getServiceWithPort(svcClusterIP, portHTTP), + endpointURL: nil, + proto: ProtocolNone, + path: "/path", + want: "foo.namespace.svc:80/path", + }, + { + name: "None ClusterIP service port 8080 override", + service: getServiceWithPort(svcClusterIP, portCustom), + endpointURL: nil, + proto: ProtocolNone, + path: "/path", + want: "foo.namespace.svc:8080/path", + }, + { + name: "Override EndpointURL with path", + service: getServiceWithPort(svcClusterIP, portCustom), + endpointURL: ptr.To("http://this.url"), + proto: ProtocolNone, + path: "/path", + want: "http://this.url/path", + }, + { + name: "Override EndpointURL no path", + service: getServiceWithPort(svcClusterIP, portCustom), + endpointURL: ptr.To("http://this.url"), + proto: ProtocolNone, + path: "", + want: "http://this.url", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + service, err := NewService(tt.service, timeout, nil) + g.Expect(err).ToNot(HaveOccurred()) + url, err := service.GetAPIEndpoint(tt.endpointURL, ptr.To(tt.proto), tt.path) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(url).To(Equal(tt.want)) + }) + } +} + +func TestToOverrideServiceSpec(t *testing.T) { + tests := []struct { + name string + service *corev1.Service + override OverrideSpec + want OverrideServiceSpec + }{ + { + name: "No override", + service: getServiceWithPort(svcClusterIP, portHTTP), + override: OverrideSpec{}, + want: overrideServiceSpecClusterIP, + }, + { + name: "HTTP ClusterIP service override service Type to LoadBalancer", + service: getServiceWithPort(svcClusterIP, portHTTP), + override: override, + want: overrideServiceSpecLoadBalancer, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + service, err := NewService(tt.service, timeout, &tt.override) + g.Expect(err).ToNot(HaveOccurred()) + // ToOverrideServiceSpec + ovrrdServiceSpec, err := service.ToOverrideServiceSpec() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(ovrrdServiceSpec).ToNot(BeNil()) + g.Expect(*ovrrdServiceSpec).To(Equal(tt.want)) + }) + } +} diff --git a/modules/common/service/types.go b/modules/common/service/types.go index e1968ace..b7b6aeb0 100644 --- a/modules/common/service/types.go +++ b/modules/common/service/types.go @@ -14,6 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ +// +kubebuilder:object:generate:=true + package service import ( @@ -22,7 +24,38 @@ import ( corev1 "k8s.io/api/core/v1" ) +// Endpoint - typedef to enumerate Endpoint verbs +// NOTE: (mschuppert) have to duplicate this for now. Can not have +// circular dep back to endpoint pkg also can not move it at this +// point as we have circular dep in test module to keystone. +type Endpoint string + +// Protocol of the endpoint (http/https) +type Protocol string + +const ( + // EndpointAdmin - admin endpoint + EndpointAdmin Endpoint = "admin" + // EndpointInternal - internal endpoint + EndpointInternal Endpoint = "internal" + // EndpointPublic - public endpoint + EndpointPublic Endpoint = "public" + // AnnotationIngressCreateKey - + AnnotationIngressCreateKey = "core.openstack.org/ingress_create" + // AnnotationEndpointKey - + AnnotationEndpointKey = "endpoint" + // AnnotationHostnameKey - + AnnotationHostnameKey = "dnsmasq.network.openstack.org/hostname" + // ProtocolHTTP - + ProtocolHTTP Protocol = "http" + // ProtocolHTTPS - + ProtocolHTTPS Protocol = "https" + // ProtocolNone - + ProtocolNone Protocol = "" +) + // Service - +// +kubebuilder:object:generate:=false type Service struct { service *corev1.Service timeout time.Duration @@ -33,16 +66,21 @@ type Service struct { } // GenericServiceDetails - +// +kubebuilder:object:generate:=false type GenericServiceDetails struct { Name string Namespace string Labels map[string]string Selector map[string]string + // deprecated, use Ports Port GenericServicePort + Ports []corev1.ServicePort ClusterIP string } // GenericServicePort - +// +kubebuilder:object:generate:=false +// NOTE: (mschuppert) deprecated, can be removed when service operators moved to Ports type GenericServicePort struct { Name string Port int32 @@ -50,13 +88,16 @@ type GenericServicePort struct { } // MetalLBServiceDetails - +// +kubebuilder:object:generate:=false type MetalLBServiceDetails struct { Name string Namespace string Annotations map[string]string Labels map[string]string Selector map[string]string - Port GenericServicePort + // deprecated, use Ports + Port GenericServicePort + Ports []corev1.ServicePort } const ( @@ -67,3 +108,134 @@ const ( // MetalLBLoadBalancerIPs - MetalLBLoadBalancerIPs = "metallb.universe.tf/loadBalancerIPs" ) + +// OverrideSpec - service override configuration for the Service created to serve traffic to the cluster. +// Allows for the manifest of the created Service to be overwritten with custom configuration. +type OverrideSpec struct { + *EmbeddedLabelsAnnotations `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` + Spec *OverrideServiceSpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"` +} + +// RoutedOverrideSpec - a routed service override configuration for the Service created to serve traffic +// to the cluster. Allows for the manifest of the created Service to be overwritten with custom configuration. +type RoutedOverrideSpec struct { + OverrideSpec `json:",inline"` + EndpointURL *string `json:"endpointURL,omitempty"` +} + +// EmbeddedLabelsAnnotations is an embedded subset of the fields included in k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta. +// Only labels and annotations are included. +type EmbeddedLabelsAnnotations struct { + // Map of string keys and values that can be used to organize and categorize + // (scope and select) objects. May match selectors of replication controllers + // and services. + // More info: http://kubernetes.io/docs/user-guide/labels + // +optional + Labels map[string]string `json:"labels,omitempty" protobuf:"bytes,11,rep,name=labels"` + + // Annotations is an unstructured key value map stored with a resource that may be + // set by external tools to store and retrieve arbitrary metadata. They are not + // queryable and should be preserved when modifying objects. + // More info: http://kubernetes.io/docs/user-guide/annotations + // +optional + Annotations map[string]string `json:"annotations,omitempty" protobuf:"bytes,12,rep,name=annotations"` +} + +// OverrideServiceSpec is a subset of the fields included in https://pkg.go.dev/k8s.io/api@v0.26.6/core/v1#ServiceSpec +// Limited to Type, SessionAffinity, LoadBalancerSourceRanges, ExternalName, ExternalTrafficPolicy, SessionAffinityConfig, +// IPFamilyPolicy, LoadBalancerClass and InternalTrafficPolicy +type OverrideServiceSpec struct { + // type determines how the Service is exposed. Defaults to ClusterIP. Valid + // options are ExternalName, ClusterIP, NodePort, and LoadBalancer. + // "ClusterIP" allocates a cluster-internal IP address for load-balancing + // to endpoints. Endpoints are determined by the selector or if that is not + // specified, by manual construction of an Endpoints object or + // EndpointSlice objects. If clusterIP is "None", no virtual IP is + // allocated and the endpoints are published as a set of endpoints rather + // than a virtual IP. + // "NodePort" builds on ClusterIP and allocates a port on every node which + // routes to the same endpoints as the clusterIP. + // "LoadBalancer" builds on NodePort and creates an external load-balancer + // (if supported in the current cloud) which routes to the same endpoints + // as the clusterIP. + // "ExternalName" aliases this service to the specified externalName. + // Several other fields do not apply to ExternalName services. + // More info: https://kubernetes.io/docs/concepts/services-networking/service/#publishing-services-service-types + // +optional + Type corev1.ServiceType `json:"type,omitempty" protobuf:"bytes,4,opt,name=type,casttype=ServiceType"` + + // Supports "ClientIP" and "None". Used to maintain session affinity. + // Enable client IP based session affinity. + // Must be ClientIP or None. + // Defaults to None. + // More info: https://kubernetes.io/docs/concepts/services-networking/service/#virtual-ips-and-service-proxies + // +optional + SessionAffinity corev1.ServiceAffinity `json:"sessionAffinity,omitempty" protobuf:"bytes,7,opt,name=sessionAffinity,casttype=ServiceAffinity"` + + // If specified and supported by the platform, this will restrict traffic through the cloud-provider + // load-balancer will be restricted to the specified client IPs. This field will be ignored if the + // cloud-provider does not support the feature." + // More info: https://kubernetes.io/docs/tasks/access-application-cluster/create-external-load-balancer/ + // +optional + LoadBalancerSourceRanges []string `json:"loadBalancerSourceRanges,omitempty" protobuf:"bytes,9,opt,name=loadBalancerSourceRanges"` + + // externalName is the external reference that discovery mechanisms will + // return as an alias for this service (e.g. a DNS CNAME record). No + // proxying will be involved. Must be a lowercase RFC-1123 hostname + // (https://tools.ietf.org/html/rfc1123) and requires `type` to be "ExternalName". + // +optional + ExternalName string `json:"externalName,omitempty" protobuf:"bytes,10,opt,name=externalName"` + + // externalTrafficPolicy describes how nodes distribute service traffic they + // receive on one of the Service's "externally-facing" addresses (NodePorts, + // ExternalIPs, and LoadBalancer IPs). If set to "Local", the proxy will configure + // the service in a way that assumes that external load balancers will take care + // of balancing the service traffic between nodes, and so each node will deliver + // traffic only to the node-local endpoints of the service, without masquerading + // the client source IP. (Traffic mistakenly sent to a node with no endpoints will + // be dropped.) The default value, "Cluster", uses the standard behavior of + // routing to all endpoints evenly (possibly modified by topology and other + // features). Note that traffic sent to an External IP or LoadBalancer IP from + // within the cluster will always get "Cluster" semantics, but clients sending to + // a NodePort from within the cluster may need to take traffic policy into account + // when picking a node. + // +optional + ExternalTrafficPolicy corev1.ServiceExternalTrafficPolicyType `json:"externalTrafficPolicy,omitempty" protobuf:"bytes,11,opt,name=externalTrafficPolicy"` + + // sessionAffinityConfig contains the configurations of session affinity. + // +optional + SessionAffinityConfig *corev1.SessionAffinityConfig `json:"sessionAffinityConfig,omitempty" protobuf:"bytes,14,opt,name=sessionAffinityConfig"` + + // IPFamilyPolicy represents the dual-stack-ness requested or required by + // this Service. If there is no value provided, then this field will be set + // to SingleStack. Services can be "SingleStack" (a single IP family), + // "PreferDualStack" (two IP families on dual-stack configured clusters or + // a single IP family on single-stack clusters), or "RequireDualStack" + // (two IP families on dual-stack configured clusters, otherwise fail). The + // ipFamilies and clusterIPs fields depend on the value of this field. This + // field will be wiped when updating a service to type ExternalName. + // +optional + IPFamilyPolicy *corev1.IPFamilyPolicy `json:"ipFamilyPolicy,omitempty" protobuf:"bytes,17,opt,name=ipFamilyPolicy,casttype=IPFamilyPolicy"` + + // loadBalancerClass is the class of the load balancer implementation this Service belongs to. + // If specified, the value of this field must be a label-style identifier, with an optional prefix, + // e.g. "internal-vip" or "example.com/internal-vip". Unprefixed names are reserved for end-users. + // This field can only be set when the Service type is 'LoadBalancer'. If not set, the default load + // balancer implementation is used, today this is typically done through the cloud provider integration, + // but should apply for any default implementation. If set, it is assumed that a load balancer + // implementation is watching for Services with a matching class. Any default load balancer + // implementation (e.g. cloud providers) should ignore Services that set this field. + // This field can only be set when creating or updating a Service to type 'LoadBalancer'. + // Once set, it can not be changed. This field will be wiped when a service is updated to a non 'LoadBalancer' type. + // +optional + LoadBalancerClass *string `json:"loadBalancerClass,omitempty" protobuf:"bytes,21,opt,name=loadBalancerClass"` + + // InternalTrafficPolicy describes how nodes distribute service traffic they + // receive on the ClusterIP. If set to "Local", the proxy will assume that pods + // only want to talk to endpoints of the service on the same node as the pod, + // dropping the traffic if there are no local endpoints. The default value, + // "Cluster", uses the standard behavior of routing to all endpoints evenly + // (possibly modified by topology and other features). + // +optional + InternalTrafficPolicy *corev1.ServiceInternalTrafficPolicyType `json:"internalTrafficPolicy,omitempty" protobuf:"bytes,22,opt,name=internalTrafficPolicy"` +} diff --git a/modules/common/service/zz_generated.deepcopy.go b/modules/common/service/zz_generated.deepcopy.go new file mode 100644 index 00000000..a0e4de0c --- /dev/null +++ b/modules/common/service/zz_generated.deepcopy.go @@ -0,0 +1,141 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +/* + + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by controller-gen. DO NOT EDIT. + +package service + +import ( + "k8s.io/api/core/v1" +) + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *EmbeddedLabelsAnnotations) DeepCopyInto(out *EmbeddedLabelsAnnotations) { + *out = *in + if in.Labels != nil { + in, out := &in.Labels, &out.Labels + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.Annotations != nil { + in, out := &in.Annotations, &out.Annotations + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EmbeddedLabelsAnnotations. +func (in *EmbeddedLabelsAnnotations) DeepCopy() *EmbeddedLabelsAnnotations { + if in == nil { + return nil + } + out := new(EmbeddedLabelsAnnotations) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OverrideServiceSpec) DeepCopyInto(out *OverrideServiceSpec) { + *out = *in + if in.LoadBalancerSourceRanges != nil { + in, out := &in.LoadBalancerSourceRanges, &out.LoadBalancerSourceRanges + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.SessionAffinityConfig != nil { + in, out := &in.SessionAffinityConfig, &out.SessionAffinityConfig + *out = new(v1.SessionAffinityConfig) + (*in).DeepCopyInto(*out) + } + if in.IPFamilyPolicy != nil { + in, out := &in.IPFamilyPolicy, &out.IPFamilyPolicy + *out = new(v1.IPFamilyPolicy) + **out = **in + } + if in.LoadBalancerClass != nil { + in, out := &in.LoadBalancerClass, &out.LoadBalancerClass + *out = new(string) + **out = **in + } + if in.InternalTrafficPolicy != nil { + in, out := &in.InternalTrafficPolicy, &out.InternalTrafficPolicy + *out = new(v1.ServiceInternalTrafficPolicyType) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OverrideServiceSpec. +func (in *OverrideServiceSpec) DeepCopy() *OverrideServiceSpec { + if in == nil { + return nil + } + out := new(OverrideServiceSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OverrideSpec) DeepCopyInto(out *OverrideSpec) { + *out = *in + if in.EmbeddedLabelsAnnotations != nil { + in, out := &in.EmbeddedLabelsAnnotations, &out.EmbeddedLabelsAnnotations + *out = new(EmbeddedLabelsAnnotations) + (*in).DeepCopyInto(*out) + } + if in.Spec != nil { + in, out := &in.Spec, &out.Spec + *out = new(OverrideServiceSpec) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OverrideSpec. +func (in *OverrideSpec) DeepCopy() *OverrideSpec { + if in == nil { + return nil + } + out := new(OverrideSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RoutedOverrideSpec) DeepCopyInto(out *RoutedOverrideSpec) { + *out = *in + in.OverrideSpec.DeepCopyInto(&out.OverrideSpec) + if in.EndpointURL != nil { + in, out := &in.EndpointURL, &out.EndpointURL + *out = new(string) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoutedOverrideSpec. +func (in *RoutedOverrideSpec) DeepCopy() *RoutedOverrideSpec { + if in == nil { + return nil + } + out := new(RoutedOverrideSpec) + in.DeepCopyInto(out) + return out +} diff --git a/modules/common/test/functional/route_test.go b/modules/common/test/functional/route_test.go index 8a2fd921..7020d661 100644 --- a/modules/common/test/functional/route_test.go +++ b/modules/common/test/functional/route_test.go @@ -74,7 +74,6 @@ var _ = Describe("route package", func() { It("creates route with defaults", func() { r, err := route.NewRoute( getExampleRoute(namespace), - map[string]string{}, timeout, &route.OverrideSpec{}, ) @@ -97,7 +96,6 @@ var _ = Describe("route package", func() { It("merges labels to the route", func() { r, err := route.NewRoute( getExampleRoute(namespace), - map[string]string{}, timeout, &route.OverrideSpec{ EmbeddedLabelsAnnotations: &route.EmbeddedLabelsAnnotations{ @@ -124,7 +122,6 @@ var _ = Describe("route package", func() { It("merges annotations to the route", func() { r, err := route.NewRoute( getExampleRoute(namespace), - map[string]string{}, timeout, &route.OverrideSpec{ EmbeddedLabelsAnnotations: &route.EmbeddedLabelsAnnotations{ @@ -151,7 +148,6 @@ var _ = Describe("route package", func() { It("overrides spec.host if specified", func() { r, err := route.NewRoute( getExampleRoute(namespace), - map[string]string{}, timeout, &route.OverrideSpec{ Spec: &route.Spec{ @@ -170,7 +166,6 @@ var _ = Describe("route package", func() { It("overrides spec.subdomain if specified", func() { r, err := route.NewRoute( getExampleRoute(namespace), - map[string]string{}, timeout, &route.OverrideSpec{ Spec: &route.Spec{ @@ -189,7 +184,6 @@ var _ = Describe("route package", func() { It("overrides spec.path if specified", func() { r, err := route.NewRoute( getExampleRoute(namespace), - map[string]string{}, timeout, &route.OverrideSpec{ Spec: &route.Spec{ @@ -208,7 +202,6 @@ var _ = Describe("route package", func() { It("overrides spec.to if specified", func() { r, err := route.NewRoute( getExampleRoute(namespace), - map[string]string{}, timeout, &route.OverrideSpec{ Spec: &route.Spec{ @@ -232,7 +225,6 @@ var _ = Describe("route package", func() { It("overrides spec.alternateBackends if specified", func() { r, err := route.NewRoute( getExampleRoute(namespace), - map[string]string{}, timeout, &route.OverrideSpec{ Spec: &route.Spec{ @@ -258,7 +250,6 @@ var _ = Describe("route package", func() { It("overrides spec.port if specified", func() { r, err := route.NewRoute( getExampleRoute(namespace), - map[string]string{}, timeout, &route.OverrideSpec{ Spec: &route.Spec{ @@ -279,7 +270,6 @@ var _ = Describe("route package", func() { It("overrides spec.tls if specified", func() { r, err := route.NewRoute( getExampleRoute(namespace), - map[string]string{}, timeout, &route.OverrideSpec{ Spec: &route.Spec{ diff --git a/modules/common/test/functional/service_test.go b/modules/common/test/functional/service_test.go new file mode 100644 index 00000000..6b847602 --- /dev/null +++ b/modules/common/test/functional/service_test.go @@ -0,0 +1,233 @@ +/* +Copyright 2023 Red Hat + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package functional + +import ( + "fmt" + + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/openstack-k8s-operators/lib-common/modules/common/service" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func getExampleService(namespace string, port int32) *corev1.Service { + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-svc", + Namespace: namespace, + Labels: map[string]string{ + "label": "a", + "replace": "a", + }, + Annotations: map[string]string{ + "anno": "a", + "replace": "a", + }, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + Ports: []corev1.ServicePort{ + { + Name: "test-svc", + Port: port, + Protocol: corev1.ProtocolTCP, + TargetPort: intstr.FromInt(8080), + }, + }, + Selector: map[string]string{ + "internal": "true", + "service": "foo", + }, + }, + } +} + +var _ = Describe("service package", func() { + var namespace string + + BeforeEach(func() { + // NOTE(gibi): We need to create a unique namespace for each test run + // as namespaces cannot be deleted in a locally running envtest. See + // https://book.kubebuilder.io/reference/envtest.html#namespace-usage-limitation + namespace = uuid.New().String() + th.CreateNamespace(namespace) + // We still request the delete of the Namespace to properly cleanup if + // we run the test in an existing cluster. + DeferCleanup(th.DeleteNamespace, namespace) + + }) + + It("creates service with defaults", func() { + s, err := service.NewService( + getExampleService(namespace, int32(80)), + timeout, + &service.OverrideSpec{}, + ) + Expect(err).ShouldNot(HaveOccurred()) + + // AddAnnotations() + s.AddAnnotation(map[string]string{"add": "bar"}) + + _, err = s.CreateOrPatch(ctx, h) + Expect(err).ShouldNot(HaveOccurred()) + svc := th.AssertServiceExists(types.NamespacedName{Namespace: namespace, Name: "test-svc"}) + Expect(svc.Annotations["anno"]).To(Equal("a")) + Expect(svc.Annotations["replace"]).To(Equal("a")) + Expect(svc.Labels["label"]).To(Equal("a")) + Expect(svc.Labels["replace"]).To(Equal("a")) + Expect(svc.Spec.Type).To(Equal(corev1.ServiceTypeClusterIP)) + Expect(svc.Spec.Ports[0].Name).To(Equal("test-svc")) + Expect(svc.Spec.Ports[0].Port).To(Equal(int32(80))) + Expect(svc.Spec.Ports[0].Protocol).To(Equal(corev1.ProtocolTCP)) + Expect(svc.Spec.Ports[0].TargetPort.IntVal).To(Equal(int32(8080))) + + // Test Getters + Expect(s.GetAnnotations()).To(Equal(map[string]string{ + "anno": "a", + "replace": "a", + "add": "bar", + })) + Expect(s.GetLabels()).To(Equal(map[string]string{ + "label": "a", + "replace": "a", + })) + Expect(s.GetServiceType()).To(Equal(corev1.ServiceTypeClusterIP)) + Expect(s.GetServiceHostname()).To(Equal(fmt.Sprintf("test-svc.%s.svc", namespace))) + hostname, port := s.GetServiceHostnamePort() + Expect(hostname).To(Equal(fmt.Sprintf("test-svc.%s.svc", namespace))) + Expect(port).To(Equal("80")) + // HTTP endpoint with no port + endpointURL, err := s.GetAPIEndpoint(nil, ptr.To(service.ProtocolHTTP), "") + Expect(err).ShouldNot(HaveOccurred()) + Expect(endpointURL).To(Equal(fmt.Sprintf("http://test-svc.%s.svc", namespace))) + + // GetServiceWithName() + svc, err = service.GetServiceWithName(th.Ctx, h, "test-svc", namespace) + Expect(err).ShouldNot(HaveOccurred()) + + // GetServicesListWithLabel() + svcList, err := service.GetServicesListWithLabel(th.Ctx, h, namespace, map[string]string{ + "label": "a", + "replace": "a", + }) + Expect(err).ShouldNot(HaveOccurred()) + Expect(svcList.Items).Should(HaveLen(1)) + + // GetServicesPortDetails() + servicePortDetails := service.GetServicesPortDetails(svc, "test-svc") + Expect(servicePortDetails).NotTo(BeNil()) + Expect(servicePortDetails.Port).To(Equal(int32(80))) + Expect(servicePortDetails.TargetPort.IntVal).To(Equal(int32(8080))) + + // Delete method + err = s.Delete(ctx, h) + Expect(err).ShouldNot(HaveOccurred()) + th.AssertServiceDoesNotExist(types.NamespacedName{Namespace: namespace, Name: "test-svc"}) + + }) + + It("merges labels to the service", func() { + s, err := service.NewService( + getExampleService(namespace, int32(5000)), + timeout, + &service.OverrideSpec{ + EmbeddedLabelsAnnotations: &service.EmbeddedLabelsAnnotations{ + Labels: map[string]string{ + "foo": "b", + "replace": "b", + }, + }, + }, + ) + Expect(err).ShouldNot(HaveOccurred()) + + _, err = s.CreateOrPatch(ctx, h) + Expect(err).ShouldNot(HaveOccurred()) + rv1 := th.AssertServiceExists(types.NamespacedName{Namespace: namespace, Name: "test-svc"}) + // non overridden label exists + Expect(rv1.Labels["label"]).To(Equal("a")) + // adds new label + Expect(rv1.Labels["foo"]).To(Equal("b")) + // override replaces existing label + Expect(rv1.Labels["replace"]).To(Equal("b")) + + // HTTPS endpoint with port + endpointURL, err := s.GetAPIEndpoint(nil, ptr.To(service.ProtocolHTTPS), "") + Expect(err).ShouldNot(HaveOccurred()) + Expect(endpointURL).To(Equal(fmt.Sprintf("https://test-svc.%s.svc:5000", namespace))) + }) + + It("merges annotations to the service", func() { + s, err := service.NewService( + getExampleService(namespace, int32(443)), + timeout, + &service.OverrideSpec{ + EmbeddedLabelsAnnotations: &service.EmbeddedLabelsAnnotations{ + Annotations: map[string]string{ + "foo": "b", + "replace": "b", + }, + }, + }, + ) + Expect(err).ShouldNot(HaveOccurred()) + + _, err = s.CreateOrPatch(ctx, h) + Expect(err).ShouldNot(HaveOccurred()) + rv1 := th.AssertServiceExists(types.NamespacedName{Namespace: namespace, Name: "test-svc"}) + // non overridden annotation exists + Expect(rv1.Annotations["anno"]).To(Equal("a")) + // adds new annotation + Expect(rv1.Annotations["foo"]).To(Equal("b")) + // override replaces existing annotation + Expect(rv1.Annotations["replace"]).To(Equal("b")) + + // HTTPS endpoint with no port + endpointURL, err := s.GetAPIEndpoint(nil, ptr.To(service.ProtocolHTTPS), "") + Expect(err).ShouldNot(HaveOccurred()) + Expect(endpointURL).To(Equal(fmt.Sprintf("https://test-svc.%s.svc", namespace))) + }) + + It("overrides spec.Type to LoadBalancer", func() { + s, err := service.NewService( + getExampleService(namespace, int32(80)), + timeout, + &service.OverrideSpec{ + Spec: &service.OverrideServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + }, + }, + ) + Expect(err).ShouldNot(HaveOccurred()) + + _, err = s.CreateOrPatch(ctx, h) + Expect(err).ShouldNot(HaveOccurred()) + svc := th.AssertServiceExists(types.NamespacedName{Namespace: namespace, Name: "test-svc"}) + Expect(svc.Spec.Type).To(Equal(corev1.ServiceTypeLoadBalancer)) + + // NONE endpoint with port + endpointURL, err := s.GetAPIEndpoint(nil, ptr.To(service.ProtocolNone), "") + Expect(err).ShouldNot(HaveOccurred()) + Expect(endpointURL).To(Equal(fmt.Sprintf("test-svc.%s.svc:80", namespace))) + }) +}) diff --git a/modules/openstack/go.mod b/modules/openstack/go.mod index 1d9c6bfd..01761a09 100644 --- a/modules/openstack/go.mod +++ b/modules/openstack/go.mod @@ -33,7 +33,6 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect - github.com/openshift/api v3.9.0+incompatible // indirect github.com/pkg/errors v0.9.1 // indirect github.com/prometheus/client_golang v1.14.0 // indirect github.com/prometheus/client_model v0.3.0 // indirect diff --git a/modules/openstack/go.sum b/modules/openstack/go.sum index 5fbf173c..866e70b7 100644 --- a/modules/openstack/go.sum +++ b/modules/openstack/go.sum @@ -222,8 +222,6 @@ github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRW github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= github.com/onsi/ginkgo/v2 v2.12.0 h1:UIVDowFPwpg6yMUpPjGkYvf06K3RAiJXUhCxEwQVHRI= github.com/onsi/gomega v1.27.10 h1:naR28SdDFlqrG6kScpT8VWpu1xWY5nJRCF3XaYyBjhI= -github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 h1:rncLxJBpFGqBztyxCMwNRnMjhhIDOWHJowi6q8G6koI= -github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= diff --git a/modules/openstack/openstack.go b/modules/openstack/openstack.go index 56ac2ade..b392f6ed 100644 --- a/modules/openstack/openstack.go +++ b/modules/openstack/openstack.go @@ -22,7 +22,7 @@ import ( "github.com/go-logr/logr" gophercloud "github.com/gophercloud/gophercloud" "github.com/gophercloud/gophercloud/openstack" - endpoint "github.com/openstack-k8s-operators/lib-common/modules/common/endpoint" + service "github.com/openstack-k8s-operators/lib-common/modules/common/service" ) // OpenStack - @@ -99,11 +99,11 @@ func GetAvailability( endpointInterface string, ) (gophercloud.Availability, error) { var availability gophercloud.Availability - if endpointInterface == string(endpoint.EndpointAdmin) { + if endpointInterface == string(service.EndpointAdmin) { availability = gophercloud.AvailabilityAdmin - } else if endpointInterface == string(endpoint.EndpointInternal) { + } else if endpointInterface == string(service.EndpointInternal) { availability = gophercloud.AvailabilityInternal - } else if endpointInterface == string(endpoint.EndpointPublic) { + } else if endpointInterface == string(service.EndpointPublic) { availability = gophercloud.AvailabilityPublic } else { return availability, fmt.Errorf("endpoint interface %s not known", endpointInterface) diff --git a/modules/test/go.mod b/modules/test/go.mod index 8067f32c..a1a06546 100644 --- a/modules/test/go.mod +++ b/modules/test/go.mod @@ -10,7 +10,7 @@ require ( github.com/onsi/gomega v1.27.10 github.com/openstack-k8s-operators/infra-operator/apis v0.1.1-0.20230914145253-116f307c7875 github.com/openstack-k8s-operators/keystone-operator/api v0.1.1-0.20230914163026-da9aa9de960a - github.com/openstack-k8s-operators/lib-common/modules/common v0.1.1-0.20230824094610-976b18ca2875 + github.com/openstack-k8s-operators/lib-common/modules/common v0.1.1-0.20230913075424-2680ce4b6ad2 github.com/openstack-k8s-operators/mariadb-operator/api v0.1.1-0.20230913081601-9e4fc8aadad5 golang.org/x/exp v0.0.0-20230905200255-921286631fa9 golang.org/x/mod v0.12.0 @@ -81,6 +81,8 @@ require ( replace github.com/openstack-k8s-operators/lib-common/modules/common => ../common +replace github.com/openstack-k8s-operators/lib-common/modules/openstack => ../openstack + // mschuppert: map to latest commit from release-4.13 tag // must consistent within modules and service operators replace github.com/openshift/api => github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 diff --git a/modules/test/go.sum b/modules/test/go.sum index 3ccde443..1435ca3a 100644 --- a/modules/test/go.sum +++ b/modules/test/go.sum @@ -234,8 +234,6 @@ github.com/openstack-k8s-operators/infra-operator/apis v0.1.1-0.20230914145253-1 github.com/openstack-k8s-operators/infra-operator/apis v0.1.1-0.20230914145253-116f307c7875/go.mod h1:NgrvT3CKMu6fE8Nt1H79qHx11L3I7Bb2eItniM7c9ow= github.com/openstack-k8s-operators/keystone-operator/api v0.1.1-0.20230914163026-da9aa9de960a h1:MFYwi2Xk9r3OMPToCSbvqYVNrm7P+aFzGDN0eVNpgu8= github.com/openstack-k8s-operators/keystone-operator/api v0.1.1-0.20230914163026-da9aa9de960a/go.mod h1:nxrbUOIGMJ1h2pNlawhURdt/XJ95dW2wZGedmOVo2aw= -github.com/openstack-k8s-operators/lib-common/modules/openstack v0.1.1-0.20230824094610-976b18ca2875 h1:aUlwELsLYWQ3FL+/nRG/1uGVNW86c3MhtLrHNVDd57k= -github.com/openstack-k8s-operators/lib-common/modules/openstack v0.1.1-0.20230824094610-976b18ca2875/go.mod h1:Vng+vqdTJUuZ+AEzSAaU0I7bn3qwYMMFEUHHhiH0440= github.com/openstack-k8s-operators/mariadb-operator/api v0.1.1-0.20230913081601-9e4fc8aadad5 h1:dQcSQuXfgzgOhc4v+zD0jE6WWhn6FHr5nALOjJBPxyI= github.com/openstack-k8s-operators/mariadb-operator/api v0.1.1-0.20230913081601-9e4fc8aadad5/go.mod h1:mJyhm/YiQZaYhLvOuLng/ITpwx8HvsYVht+VotS1Ed8= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=