From ba13acb16b5eed9ffe77be9a75dd71ec41f57243 Mon Sep 17 00:00:00 2001 From: ssttehrani Date: Sat, 6 Apr 2024 17:47:57 +0330 Subject: [PATCH 1/4] fix: ingress packet drop issue for non-vpn-accessible services --- internal/controller/service/helpers.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/internal/controller/service/helpers.go b/internal/controller/service/helpers.go index a6123ed..23ba13c 100644 --- a/internal/controller/service/helpers.go +++ b/internal/controller/service/helpers.go @@ -51,17 +51,9 @@ var ( // buildCNP builds and returns a CiliumNetworkPolicy struct based on the Service object. func (re *ReconcilerExtended) buildCNP() *ciliumv2.CiliumNetworkPolicy { - var fromEntities cilium_policy_api.EntitySlice - - if re.service.Annotations["metallb.universe.tf/address-pool"] == "vpn-access" { - fromEntities = cilium_policy_api.EntitySlice{ - cilium_policy_api.EntityCluster, - cilium_policy_api.EntityWorld, - } - } else { - fromEntities = cilium_policy_api.EntitySlice{ - cilium_policy_api.EntityCluster, - } + fromEntities := cilium_policy_api.EntitySlice{ + cilium_policy_api.EntityCluster, + cilium_policy_api.EntityWorld, } endpointSelectorLabelsMap := make(map[string]string, len(re.service.Spec.Selector)) From 233c497f5c28a7c8980e62df3106ac7fee4720d7 Mon Sep 17 00:00:00 2001 From: ssttehrani Date: Sat, 6 Apr 2024 17:51:04 +0330 Subject: [PATCH 2/4] chore: fix linting issues --- internal/controller/service/controller.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/controller/service/controller.go b/internal/controller/service/controller.go index c27cff8..83981dd 100644 --- a/internal/controller/service/controller.go +++ b/internal/controller/service/controller.go @@ -245,9 +245,11 @@ func (re *ReconcilerExtended) addFinalizer(ctx context.Context) (*ctrl.Result, e if err := re.Client.Get(ctx, re.request.NamespacedName, re.service); err != nil { return err } + if controllerutil.ContainsFinalizer(re.service, consts.ServiceFinalizerString) { return nil } + controllerutil.AddFinalizer(re.service, consts.ServiceFinalizerString) return re.Client.Update(ctx, re.service) @@ -268,9 +270,11 @@ func (re *ReconcilerExtended) removeFinalizer(ctx context.Context) (*ctrl.Result if err := re.Client.Get(ctx, re.request.NamespacedName, re.service); err != nil { return err } + if !controllerutil.ContainsFinalizer(re.service, consts.ServiceFinalizerString) { return nil } + controllerutil.RemoveFinalizer(re.service, consts.ServiceFinalizerString) return re.Client.Update(ctx, re.service) From 2eeb122a7742b9a4629d7e31fdf51834b2cc2b27 Mon Sep 17 00:00:00 2001 From: ssttehrani Date: Sat, 6 Apr 2024 18:02:45 +0330 Subject: [PATCH 3/4] fix: test cases --- internal/controller/controller_test.go | 63 +++++--------------------- 1 file changed, 11 insertions(+), 52 deletions(-) diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go index 55aa4e5..a28b711 100644 --- a/internal/controller/controller_test.go +++ b/internal/controller/controller_test.go @@ -50,15 +50,14 @@ const ( // Global variables for test configuration var ( - CiliumNetworkPolicyControllerLabels map[string]string = map[string]string{"snappcloud.io/controller-managed": "true", "snappcloud.io/controller": "svc-lb-to-cilium-netpolicy"} - CiliumNetworkPolicyAllowedEntities cilium_policy_api.EntitySlice = []cilium_policy_api.Entity{cilium_policy_api.EntityCluster} - CiliumNetworkPolicyAllowedEntitiesForExposedServices cilium_policy_api.EntitySlice = []cilium_policy_api.Entity{cilium_policy_api.EntityCluster, cilium_policy_api.EntityWorld} - CiliumNetworkPolicyAnnotations map[string]string = map[string]string{"snappcloud.io/team": "snappcloud", "snappcloud.io/sub-team": "network"} - ExcludedNamespaceLabelsViaLabelsSet map[string]string = map[string]string{"k1": "v1", "k2": "v2"} // Set based on the configuration - MetallbAnnotations map[string]string = map[string]string{"metallb.universe.tf/address-pool": "vpn-access"} - ServiceLabels map[string]string = map[string]string{"app.kubernetes.io/name": "test", "k2": "v2"} - ServicePorts []corev1.ServicePort = []corev1.ServicePort{{Name: "http", Port: 80}} - ServiceSelector map[string]string = map[string]string{"app.kubernetes.io/name": "test", "k2": "v2"} + CiliumNetworkPolicyControllerLabels map[string]string = map[string]string{"snappcloud.io/controller-managed": "true", "snappcloud.io/controller": "svc-lb-to-cilium-netpolicy"} + CiliumNetworkPolicyAllowedEntities cilium_policy_api.EntitySlice = []cilium_policy_api.Entity{cilium_policy_api.EntityCluster, cilium_policy_api.EntityWorld} + CiliumNetworkPolicyAnnotations map[string]string = map[string]string{"snappcloud.io/team": "snappcloud", "snappcloud.io/sub-team": "network"} + ExcludedNamespaceLabelsViaLabelsSet map[string]string = map[string]string{"k1": "v1", "k2": "v2"} // Set based on the configuration + MetallbAnnotations map[string]string = map[string]string{"metallb.universe.tf/address-pool": "vpn-access"} + ServiceLabels map[string]string = map[string]string{"app.kubernetes.io/name": "test", "k2": "v2"} + ServicePorts []corev1.ServicePort = []corev1.ServicePort{{Name: "http", Port: 80}} + ServiceSelector map[string]string = map[string]string{"app.kubernetes.io/name": "test", "k2": "v2"} ) // Main block for testing 'LoadBalancer Service' to CiliumNetworkPolicy controller @@ -122,23 +121,6 @@ var _ = Describe("Testing LoadBalancer Service to CiliumNetworkPolicy Controller } } - // Utility function to get a sample exposed Service - getSampleExposedService := func(name, namespace string) *corev1.Service { - return &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: name, - Labels: ServiceLabels, - Annotations: MetallbAnnotations, - }, - Spec: corev1.ServiceSpec{ - Ports: ServicePorts, - Type: corev1.ServiceTypeLoadBalancer, - Selector: ServiceSelector, - }, - } - } - // Utility function to delete a Service deleteService := func(service *corev1.Service) { Expect(k8sClient.Delete(context.Background(), service)).To(Succeed()) @@ -621,8 +603,8 @@ var _ = Describe("Testing LoadBalancer Service to CiliumNetworkPolicy Controller deleteService(serviceObj) }) - // Test case: Verifies that the CiliumNetworkPolicy object's Spec.Ingress[0].FromEntities are set based on the CiliumNetworkPolicyAllowedEntities variable for non-exposed services - It("should set CiliumNetworkPolicy object \"Spec.Ingress[0].FromEntities\" based on CiliumNetworkPolicyAllowedEntities variable if the Service is not exposed", func() { + // Test case: Verifies that the CiliumNetworkPolicy object's Spec.Ingress[0].FromEntities are set based on the CiliumNetworkPolicyAllowedEntities variable + It("should set CiliumNetworkPolicy object \"Spec.Ingress[0].FromEntities\" based on CiliumNetworkPolicyAllowedEntities variable", func() { // Create and verify a sample LoadBalancer Service in the default namespace serviceObj := getSampleService(ServiceName, DefaultNamespace) Expect(k8sClient.Create(context.Background(), serviceObj)).To(Succeed()) @@ -634,7 +616,7 @@ var _ = Describe("Testing LoadBalancer Service to CiliumNetworkPolicy Controller // Wait for a specified interval to ensure the state is stable time.Sleep(WaitInterval * time.Second) - // Retrieve the created CiliumNetworkPolicy object and verify its Ingress FromEntities + // Retrieve the created CiliumNetworkPolicy object and verify its Ingress FromEntities ciliumNetworkPolicyObj := ciliumv2.CiliumNetworkPolicy{} Expect(k8sClient.Get(context.Background(), types.NamespacedName{Namespace: DefaultNamespace, Name: CiliumNetworkPolicyName}, &ciliumNetworkPolicyObj)).To(Succeed()) Expect(ciliumNetworkPolicyObj.Spec.Ingress[0].FromEntities).To(Equal(CiliumNetworkPolicyAllowedEntities)) @@ -644,29 +626,6 @@ var _ = Describe("Testing LoadBalancer Service to CiliumNetworkPolicy Controller deleteService(serviceObj) }) - // Test case: Verifies that the CiliumNetworkPolicy object's Spec.Ingress[0].FromEntities are set based on the CiliumNetworkPolicyAllowedEntitiesForExposedServices variable for exposed services - It("should set CiliumNetworkPolicy object \"Spec.Ingress[0].FromEntities\" based on CiliumNetworkPolicyAllowedEntities variable if the Service is exposed", func() { - // Create and verify a sample LoadBalancer Service in the default namespace - serviceObj := getSampleExposedService(ServiceName, DefaultNamespace) - Expect(k8sClient.Create(context.Background(), serviceObj)).To(Succeed()) - - // Create a dummy CiliumNetworkPolicy object to bypass the 'NoPolicyRule' rule and verify its creation - dummyCiliumNetworkPolicyObj := getSampleCiliumNetworkPolicy(DummyName, DefaultNamespace) - Expect(k8sClient.Create(context.Background(), dummyCiliumNetworkPolicyObj)).To(Succeed()) - - // Wait for a specified interval to ensure the state is stable - time.Sleep(WaitInterval * time.Second) - - // Retrieve the created CiliumNetworkPolicy object and verify its Ingress FromEntities for exposed services - ciliumNetworkPolicyObj := ciliumv2.CiliumNetworkPolicy{} - Expect(k8sClient.Get(context.Background(), types.NamespacedName{Namespace: DefaultNamespace, Name: CiliumNetworkPolicyName}, &ciliumNetworkPolicyObj)).To(Succeed()) - Expect(ciliumNetworkPolicyObj.Spec.Ingress[0].FromEntities).To(Equal(CiliumNetworkPolicyAllowedEntitiesForExposedServices)) - - // Cleanup: Delete the dummy CiliumNetworkPolicy and Service objects - deleteCiliumNetworkPolicy(dummyCiliumNetworkPolicyObj) - deleteService(serviceObj) - }) - // Test case: Verifies that a CiliumNetworkPolicy object's annotations are set based on a predefined variable It("should set CiliumNetworkPolicy object 'ObjectMeta.Annotations' based on CiliumNetworkPolicyAnnotations variable", func() { // Create and verify a sample LoadBalancer Service in the default namespace From 17d6576384e0faaab74a7b77736c092d5d5f7dee Mon Sep 17 00:00:00 2001 From: ssttehrani Date: Sat, 6 Apr 2024 19:14:35 +0330 Subject: [PATCH 4/4] fix: controller-gen panic bug --- Makefile | 2 +- config/rbac/role.yaml | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Makefile b/Makefile index d201a07..d1c7e5e 100644 --- a/Makefile +++ b/Makefile @@ -184,7 +184,7 @@ ENVTEST ?= $(LOCALBIN)/setup-envtest ## Tool Versions KUSTOMIZE_VERSION ?= v4.5.7 -CONTROLLER_TOOLS_VERSION ?= v0.11.1 +CONTROLLER_TOOLS_VERSION ?= v0.14.0 KUSTOMIZE_INSTALL_SCRIPT ?= "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh" .PHONY: kustomize diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 73ea364..6fdc852 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -2,7 +2,6 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: - creationTimestamp: null name: manager-role rules: - apiGroups: