From 60ab33337d60ca582ccb940864e532191767b6b8 Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Tue, 17 Aug 2021 15:18:09 +0200 Subject: [PATCH] feat: enforcement of LoadBalancer service kind --- api/v1alpha1/conversion_hub.go | 18 +++++ api/v1alpha1/conversion_hub_test.go | 2 + api/v1beta1/service_allowed_types.go | 3 + api/v1beta1/zz_generated.deepcopy.go | 5 ++ e2e/disable_loadbalancer_test.go | 112 +++++++++++++++++++++++++++ e2e/enable_loadbalancer_test.go | 84 ++++++++++++++++++++ pkg/webhook/service/errors.go | 10 +++ pkg/webhook/service/validating.go | 8 ++ 8 files changed, 242 insertions(+) create mode 100644 e2e/disable_loadbalancer_test.go create mode 100644 e2e/enable_loadbalancer_test.go diff --git a/api/v1alpha1/conversion_hub.go b/api/v1alpha1/conversion_hub.go index d77b6f53..bff22ae8 100644 --- a/api/v1alpha1/conversion_hub.go +++ b/api/v1alpha1/conversion_hub.go @@ -26,6 +26,7 @@ const ( enableNodePortsAnnotation = "capsule.clastix.io/enable-node-ports" enableExternalNameAnnotation = "capsule.clastix.io/enable-external-name" + enableLoadBalancerAnnotation = "capsule.clastix.io/enable-loadbalancer-service" ownerGroupsAnnotation = "owners.capsule.clastix.io/group" ownerUsersAnnotation = "owners.capsule.clastix.io/user" @@ -297,6 +298,21 @@ func (t *Tenant) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.ServiceOptions.AllowedServices.ExternalName = pointer.BoolPtr(val) } + loadBalancerService, ok := annotations[enableLoadBalancerAnnotation] + if ok { + val, err := strconv.ParseBool(loadBalancerService) + if err != nil { + return errors.Wrap(err, fmt.Sprintf("unable to parse %s annotation on tenant %s", enableLoadBalancerAnnotation, t.GetName())) + } + if dst.Spec.ServiceOptions == nil { + dst.Spec.ServiceOptions = &capsulev1beta1.ServiceOptions{} + } + if dst.Spec.ServiceOptions.AllowedServices == nil { + dst.Spec.ServiceOptions.AllowedServices = &capsulev1beta1.AllowedServices{} + } + dst.Spec.ServiceOptions.AllowedServices.LoadBalancer = pointer.BoolPtr(val) + } + // Status dst.Status = capsulev1beta1.TenantStatus{ Size: t.Status.Size, @@ -309,6 +325,7 @@ func (t *Tenant) ConvertTo(dstRaw conversion.Hub) error { delete(dst.ObjectMeta.Annotations, podPriorityAllowedRegexAnnotation) delete(dst.ObjectMeta.Annotations, enableNodePortsAnnotation) delete(dst.ObjectMeta.Annotations, enableExternalNameAnnotation) + delete(dst.ObjectMeta.Annotations, enableLoadBalancerAnnotation) delete(dst.ObjectMeta.Annotations, ownerGroupsAnnotation) delete(dst.ObjectMeta.Annotations, ownerUsersAnnotation) delete(dst.ObjectMeta.Annotations, ownerServiceAccountAnnotation) @@ -530,6 +547,7 @@ func (t *Tenant) ConvertFrom(srcRaw conversion.Hub) error { if src.Spec.ServiceOptions != nil && src.Spec.ServiceOptions.AllowedServices != nil { t.Annotations[enableNodePortsAnnotation] = strconv.FormatBool(*src.Spec.ServiceOptions.AllowedServices.NodePort) t.Annotations[enableExternalNameAnnotation] = strconv.FormatBool(*src.Spec.ServiceOptions.AllowedServices.ExternalName) + t.Annotations[enableLoadBalancerAnnotation] = strconv.FormatBool(*src.Spec.ServiceOptions.AllowedServices.LoadBalancer) } // Status diff --git a/api/v1alpha1/conversion_hub_test.go b/api/v1alpha1/conversion_hub_test.go index 663933ca..a9c8cac3 100644 --- a/api/v1alpha1/conversion_hub_test.go +++ b/api/v1alpha1/conversion_hub_test.go @@ -52,6 +52,7 @@ func generateTenantsSpecs() (Tenant, capsulev1beta1.Tenant) { AllowedServices: &capsulev1beta1.AllowedServices{ NodePort: pointer.BoolPtr(false), ExternalName: pointer.BoolPtr(false), + LoadBalancer: pointer.BoolPtr(false), }, ExternalServiceIPs: &capsulev1beta1.ExternalServiceIPsSpec{ Allowed: []capsulev1beta1.AllowedIP{"192.168.0.1"}, @@ -285,6 +286,7 @@ func generateTenantsSpecs() (Tenant, capsulev1beta1.Tenant) { podAllowedImagePullPolicyAnnotation: "Always,IfNotPresent", enableExternalNameAnnotation: "false", enableNodePortsAnnotation: "false", + enableLoadBalancerAnnotation: "false", podPriorityAllowedAnnotation: "default", podPriorityAllowedRegexAnnotation: "^tier-.*$", ownerGroupsAnnotation: "owner-foo,owner-bar", diff --git a/api/v1beta1/service_allowed_types.go b/api/v1beta1/service_allowed_types.go index 98732330..38e692b5 100644 --- a/api/v1beta1/service_allowed_types.go +++ b/api/v1beta1/service_allowed_types.go @@ -10,4 +10,7 @@ type AllowedServices struct { //+kubebuilder:default=true // Specifies if ExternalName service type resources are allowed for the Tenant. Default is true. Optional. ExternalName *bool `json:"externalName,omitempty"` + //+kubebuilder:default=true + // Specifies if LoadBalancer service type resources are allowed for the Tenant. Default is true. Optional. + LoadBalancer *bool `json:"loadBalancer,omitempty"` } diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 6260ba75..225107a4 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -96,6 +96,11 @@ func (in *AllowedServices) DeepCopyInto(out *AllowedServices) { *out = new(bool) **out = **in } + if in.LoadBalancer != nil { + in, out := &in.LoadBalancer, &out.LoadBalancer + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AllowedServices. diff --git a/e2e/disable_loadbalancer_test.go b/e2e/disable_loadbalancer_test.go new file mode 100644 index 00000000..5b0f03ab --- /dev/null +++ b/e2e/disable_loadbalancer_test.go @@ -0,0 +1,112 @@ +//+build e2e + +// Copyright 2020-2021 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "context" + + . "github.com/onsi/ginkgo" + . "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/pointer" + + capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" +) + +var _ = Describe("creating a LoadBalancer service when it is disabled for Tenant", func() { + tnt := &capsulev1beta1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "disable-loadbalancer-service", + }, + Spec: capsulev1beta1.TenantSpec{ + Owners: capsulev1beta1.OwnerListSpec{ + { + Name: "amazon", + Kind: "User", + }, + }, + ServiceOptions: &capsulev1beta1.ServiceOptions{ + AllowedServices: &capsulev1beta1.AllowedServices{ + LoadBalancer: pointer.BoolPtr(false), + }, + }, + }, + } + + JustBeforeEach(func() { + EventuallyCreation(func() error { + return k8sClient.Create(context.TODO(), tnt) + }).Should(Succeed()) + }) + + JustAfterEach(func() { + Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed()) + }) + + It("should fail creating a service with LoadBalancer type", func() { + ns := NewNamespace("disable-loadbalancer-service") + + NamespaceCreation(ns, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) + + EventuallyCreation(func() error { + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-ip", + Namespace: ns.GetName(), + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + Ports: []corev1.ServicePort{ + { + Port: 8888, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8888, + }, + Protocol: corev1.ProtocolTCP, + }, + }, + }, + } + + cs := ownerClient(tnt.Spec.Owners[0]) + + _, err := cs.CoreV1().Services(ns.Name).Create(context.Background(), svc, metav1.CreateOptions{}) + + return err + }).Should(Succeed()) + + EventuallyCreation(func() error { + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "disable-loadbalancer-service", + Namespace: ns.GetName(), + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + Ports: []corev1.ServicePort{ + { + Port: 9999, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 9999, + }, + Protocol: corev1.ProtocolTCP, + }, + }, + }, + } + + cs := ownerClient(tnt.Spec.Owners[0]) + + _, err := cs.CoreV1().Services(ns.Name).Create(context.Background(), svc, metav1.CreateOptions{}) + + return err + }).ShouldNot(Succeed()) + }) +}) diff --git a/e2e/enable_loadbalancer_test.go b/e2e/enable_loadbalancer_test.go new file mode 100644 index 00000000..e9bc69e5 --- /dev/null +++ b/e2e/enable_loadbalancer_test.go @@ -0,0 +1,84 @@ +//+build e2e + +// Copyright 2020-2021 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "context" + + . "github.com/onsi/ginkgo" + . "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/pointer" + + capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" +) + +var _ = Describe("creating a LoadBalancer service when it is enabled for Tenant", func() { + tnt := &capsulev1beta1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "enable-loadbalancer-service", + }, + Spec: capsulev1beta1.TenantSpec{ + Owners: capsulev1beta1.OwnerListSpec{ + { + Name: "netflix", + Kind: "User", + }, + }, + ServiceOptions: &capsulev1beta1.ServiceOptions{ + AllowedServices: &capsulev1beta1.AllowedServices{ + LoadBalancer: pointer.BoolPtr(true), + }, + }, + }, + } + + JustBeforeEach(func() { + EventuallyCreation(func() error { + return k8sClient.Create(context.TODO(), tnt) + }).Should(Succeed()) + }) + + JustAfterEach(func() { + Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed()) + }) + + It("should succeed creating a service with LoadBalancer type", func() { + ns := NewNamespace("enable-loadbalancer-service") + + NamespaceCreation(ns, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed()) + + EventuallyCreation(func() error { + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "enable-loadbalancer-service", + Namespace: ns.GetName(), + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + Ports: []corev1.ServicePort{ + { + Port: 9999, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 9999, + }, + Protocol: corev1.ProtocolTCP, + }, + }, + }, + } + + cs := ownerClient(tnt.Spec.Owners[0]) + + _, err := cs.CoreV1().Services(ns.Name).Create(context.Background(), svc, metav1.CreateOptions{}) + + return err + }).Should(Succeed()) + }) +}) diff --git a/pkg/webhook/service/errors.go b/pkg/webhook/service/errors.go index 098809a0..4a106f33 100644 --- a/pkg/webhook/service/errors.go +++ b/pkg/webhook/service/errors.go @@ -51,3 +51,13 @@ func NewExternalNameDisabledError() error { func (externalNameDisabled) Error() string { return "ExternalName service types are forbidden for the tenant: please, reach out to the system administrators" } + +type loadBalancerDisabled struct{} + +func NewLoadBalancerDisabled() error { + return &loadBalancerDisabled{} +} + +func (loadBalancerDisabled) Error() string { + return "LoadBalancer service types are forbidden for the tenant: please, reach out to the system administrators" +} diff --git a/pkg/webhook/service/validating.go b/pkg/webhook/service/validating.go index ea9d52f9..83b721a4 100644 --- a/pkg/webhook/service/validating.go +++ b/pkg/webhook/service/validating.go @@ -60,6 +60,14 @@ func (r *handler) handleService(ctx context.Context, clt client.Client, decoder return &response } + if svc.Spec.Type == corev1.ServiceTypeLoadBalancer && tnt.Spec.ServiceOptions != nil && tnt.Spec.ServiceOptions.AllowedServices != nil && !*tnt.Spec.ServiceOptions.AllowedServices.LoadBalancer { + recorder.Eventf(&tnt, corev1.EventTypeWarning, "ForbiddenLoadBalancer", "Service %s/%s cannot be type of LoadBalancer for the current Tenant", req.Namespace, req.Name) + + response := admission.Denied(NewLoadBalancerDisabled().Error()) + + return &response + } + if svc.Spec.ExternalIPs == nil || (tnt.Spec.ServiceOptions == nil || tnt.Spec.ServiceOptions.ExternalServiceIPs == nil) { return nil }