From 84c2fac91b40cafbc4a2832a4d41463a4e37e764 Mon Sep 17 00:00:00 2001 From: Grant Linville Date: Wed, 29 Mar 2023 17:28:52 -0400 Subject: [PATCH 1/7] Handle case where an Ingress routes to an ExternalName Service Signed-off-by: Grant Linville --- pkg/controller/appdefinition/networkpolicy.go | 35 ++++++++++++++++++- pkg/controller/routes.go | 1 + 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/pkg/controller/appdefinition/networkpolicy.go b/pkg/controller/appdefinition/networkpolicy.go index 6189ce743..049a0cd1f 100644 --- a/pkg/controller/appdefinition/networkpolicy.go +++ b/pkg/controller/appdefinition/networkpolicy.go @@ -3,6 +3,7 @@ package appdefinition import ( "fmt" "strconv" + "strings" v1 "github.com/acorn-io/acorn/pkg/apis/internal.acorn.io/v1" "github.com/acorn-io/acorn/pkg/config" @@ -72,6 +73,12 @@ func NetworkPolicyForApp(req router.Request, resp router.Response) error { // Acorn apps from the ingress controller. If the ingress controller namespace is not defined, traffic from // all namespaces will be allowed instead. func NetworkPolicyForIngress(req router.Request, resp router.Response) error { + // check if this is being called as a finalizer for a deleted Ingress + // we need this because we sometimes create NetworkPolicies in different namespaces from where their owning Ingresses live + if !req.Object.GetDeletionTimestamp().IsZero() { + return nil + } + cfg, err := config.Get(req.Ctx, req.Client) if err != nil { return err @@ -106,6 +113,32 @@ func NetworkPolicyForIngress(req router.Request, resp router.Response) error { return err } + // This service is either a normal ClusterIP service or an ExternalName service which + // points to a service in a different namespace (if there are Acorn links involved). + // If it's an ExternalName, we need to get the service to which it points. + if svc.Spec.Type == corev1.ServiceTypeExternalName { + externalName := svc.Spec.ExternalName + + // the ExternalName is in the format ..svc. + svcName, rest, ok := strings.Cut(externalName, ".") + if !ok { + return fmt.Errorf("failed to parse ExternalName '%s' of svc '%s'", externalName, svc.Name) + } + svcNamespace, _, ok := strings.Cut(rest, ".") + if !ok { + return fmt.Errorf("failed to parse ExternalName '%s' of svc '%s'", externalName, svc.Name) + } + + svc = corev1.Service{} + err = req.Get(&svc, svcNamespace, svcName) + if err != nil { + if apierror.IsNotFound(err) { + return fmt.Errorf("failed to find service '%s', targeted by ExternalName '%s'", svcName, externalName) + } + return err + } + } + netPolName := name.SafeConcatName(projectName, appName, ingress.Name, svcName) // build the namespaceSelector for the NetPol @@ -143,7 +176,7 @@ func NetworkPolicyForIngress(req router.Request, resp router.Response) error { resp.Objects(&networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ Name: netPolName, - Namespace: ingress.Namespace, + Namespace: svc.Namespace, }, Spec: networkingv1.NetworkPolicySpec{ PodSelector: metav1.LabelSelector{ diff --git a/pkg/controller/routes.go b/pkg/controller/routes.go index 6a205651b..87857b577 100644 --- a/pkg/controller/routes.go +++ b/pkg/controller/routes.go @@ -88,6 +88,7 @@ func routes(router *router.Router, registryTransport http.RoundTripper) { router.Type(&storagev1.StorageClass{}).HandlerFunc(volume.SyncVolumeClasses) router.Type(&corev1.Service{}).Selector(managedSelector).HandlerFunc(appdefinition.NetworkPolicyForService) router.Type(&netv1.Ingress{}).Selector(managedSelector).HandlerFunc(appdefinition.NetworkPolicyForIngress) + router.Type(&netv1.Ingress{}).Selector(managedSelector).FinalizeFunc("acorn.io/netpol", appdefinition.NetworkPolicyForIngress) configRouter := router.Type(&corev1.ConfigMap{}).Namespace(system.Namespace).Name(system.ConfigName) configRouter.Handler(config.NewDNSConfigHandler()) From 8daed00c9b7d1c5509ebce9c92af88e1769d529c Mon Sep 17 00:00:00 2001 From: Grant Linville Date: Wed, 29 Mar 2023 22:14:20 -0400 Subject: [PATCH 2/7] Add test for ExternalName part of NetworkPolicyForIngress Signed-off-by: Grant Linville --- .../appdefinition/networkpolicy_test.go | 4 ++ .../networkpolicy/externalname/existing.yaml | 44 +++++++++++++++++++ .../networkpolicy/externalname/expected.yaml | 26 +++++++++++ .../networkpolicy/externalname/input.yaml | 22 ++++++++++ 4 files changed, 96 insertions(+) create mode 100644 pkg/controller/appdefinition/testdata/networkpolicy/externalname/existing.yaml create mode 100644 pkg/controller/appdefinition/testdata/networkpolicy/externalname/expected.yaml create mode 100644 pkg/controller/appdefinition/testdata/networkpolicy/externalname/input.yaml diff --git a/pkg/controller/appdefinition/networkpolicy_test.go b/pkg/controller/appdefinition/networkpolicy_test.go index 68c2f8ee6..cba9c6218 100644 --- a/pkg/controller/appdefinition/networkpolicy_test.go +++ b/pkg/controller/appdefinition/networkpolicy_test.go @@ -15,6 +15,10 @@ func TestNetworkPolicyForIngress(t *testing.T) { tester.DefaultTest(t, scheme.Scheme, "testdata/networkpolicy/ingress", NetworkPolicyForIngress) } +func TestNetworkPolicyForIngressExternalName(t *testing.T) { + tester.DefaultTest(t, scheme.Scheme, "testdata/networkpolicy/externalname", NetworkPolicyForIngress) +} + func TestNetworkPolicyForService(t *testing.T) { tester.DefaultTest(t, scheme.Scheme, "testdata/networkpolicy/service", NetworkPolicyForService) } diff --git a/pkg/controller/appdefinition/testdata/networkpolicy/externalname/existing.yaml b/pkg/controller/appdefinition/testdata/networkpolicy/externalname/existing.yaml new file mode 100644 index 000000000..541d09209 --- /dev/null +++ b/pkg/controller/appdefinition/testdata/networkpolicy/externalname/existing.yaml @@ -0,0 +1,44 @@ +--- +apiVersion: v1 +data: + config: '{"ingressControllerNamespace":"traefik"}' +kind: ConfigMap +metadata: + name: acorn-config + namespace: acorn-system +--- +apiVersion: v1 +kind: Service +metadata: + name: service-7777 + namespace: my-app-namespace + labels: + acorn.io/service-name: service-7777 +spec: + type: ClusterIP + ports: + - name: "7777" + port: 7777 + protocol: TCP + targetPort: 9999 + selector: + acorn.io/app-name: my-app + acorn.io/app-namespace: acorn + acorn.io/managed: "true" + port-number.acorn.io/9999: "true" + service-name.acorn.io/service-7777: "true" +--- +apiVersion: v1 +kind: Service +metadata: + name: service-7777 + namespace: other-namespace +spec: + type: ExternalName + externalName: service-7777.my-app-namespace.svc.cluster.local + ports: + - appProtocol: HTTP + name: "7777" + port: 7777 + protocol: TCP + targetPort: 7777 diff --git a/pkg/controller/appdefinition/testdata/networkpolicy/externalname/expected.yaml b/pkg/controller/appdefinition/testdata/networkpolicy/externalname/expected.yaml new file mode 100644 index 000000000..345e9575b --- /dev/null +++ b/pkg/controller/appdefinition/testdata/networkpolicy/externalname/expected.yaml @@ -0,0 +1,26 @@ +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: acorn-my-app-service-7777-service-7777-9999 + namespace: my-app-namespace +spec: + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: traefik + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: acorn-system + ports: + - port: 9999 + protocol: TCP + podSelector: + matchLabels: + acorn.io/app-name: my-app + acorn.io/app-namespace: acorn + acorn.io/managed: "true" + port-number.acorn.io/9999: "true" + service-name.acorn.io/service-7777: "true" + policyTypes: + - Ingress diff --git a/pkg/controller/appdefinition/testdata/networkpolicy/externalname/input.yaml b/pkg/controller/appdefinition/testdata/networkpolicy/externalname/input.yaml new file mode 100644 index 000000000..1b2100399 --- /dev/null +++ b/pkg/controller/appdefinition/testdata/networkpolicy/externalname/input.yaml @@ -0,0 +1,22 @@ +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + labels: + acorn.io/app-name: my-app + acorn.io/app-namespace: acorn + acorn.io/managed: "true" + acorn.io/service-name: my-service + name: service-7777 + namespace: other-namespace +spec: + rules: + - host: myhostname.on-acorn.io + http: + paths: + - backend: + service: + name: service-7777 + port: + number: 7777 + path: /seven + pathType: Prefix From df38e39182b104c68ff8820d7f5191503a1f8026 Mon Sep 17 00:00:00 2001 From: Grant Linville Date: Wed, 29 Mar 2023 22:31:46 -0400 Subject: [PATCH 3/7] Slight code cleanup Signed-off-by: Grant Linville --- pkg/controller/appdefinition/networkpolicy.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/controller/appdefinition/networkpolicy.go b/pkg/controller/appdefinition/networkpolicy.go index 049a0cd1f..20c429602 100644 --- a/pkg/controller/appdefinition/networkpolicy.go +++ b/pkg/controller/appdefinition/networkpolicy.go @@ -130,8 +130,7 @@ func NetworkPolicyForIngress(req router.Request, resp router.Response) error { } svc = corev1.Service{} - err = req.Get(&svc, svcNamespace, svcName) - if err != nil { + if err = req.Get(&svc, svcNamespace, svcName); err != nil { if apierror.IsNotFound(err) { return fmt.Errorf("failed to find service '%s', targeted by ExternalName '%s'", svcName, externalName) } From 66ade3156d44373b9abc5b9f2f562c430356e555 Mon Sep 17 00:00:00 2001 From: Grant Linville Date: Fri, 31 Mar 2023 21:32:48 -0400 Subject: [PATCH 4/7] Swap finalizer for garbage collection Signed-off-by: Grant Linville --- integration/run/run_test.go | 8 ++++++++ pkg/controller/appdefinition/networkpolicy.go | 9 +++++++++ 2 files changed, 17 insertions(+) diff --git a/integration/run/run_test.go b/integration/run/run_test.go index ca24f536c..65c698dd2 100644 --- a/integration/run/run_test.go +++ b/integration/run/run_test.go @@ -14,6 +14,7 @@ import ( adminv1 "github.com/acorn-io/acorn/pkg/apis/internal.admin.acorn.io/v1" "github.com/acorn-io/acorn/pkg/appdefinition" "github.com/acorn-io/acorn/pkg/client" + "github.com/acorn-io/acorn/pkg/config" kclient "github.com/acorn-io/acorn/pkg/k8sclient" "github.com/acorn-io/acorn/pkg/labels" "github.com/acorn-io/acorn/pkg/run" @@ -1150,6 +1151,13 @@ func TestCrossProjectNetworkConnection(t *testing.T) { c, _ := helper.ClientAndNamespace(t) kc := helper.MustReturn(kclient.Default) + cfg, err := config.Get(ctx, kc) + if err != nil { + t.Fatal(err) + } else if !*cfg.NetworkPolicies { + t.SkipNow() // skip this test because NetworkPolicies are not enabled + } + // create two separate projects in which to run two Nginx apps proj1, err := c.ProjectCreate(ctx, "proj1", "local") if err != nil { diff --git a/pkg/controller/appdefinition/networkpolicy.go b/pkg/controller/appdefinition/networkpolicy.go index 20c429602..64b9d90f3 100644 --- a/pkg/controller/appdefinition/networkpolicy.go +++ b/pkg/controller/appdefinition/networkpolicy.go @@ -55,6 +55,9 @@ func NetworkPolicyForApp(req router.Request, resp router.Response) error { ObjectMeta: metav1.ObjectMeta{ Name: app.Name, Namespace: podNamespace, + Labels: map[string]string{ + labels.AcornManaged: "true", + }, }, Spec: networkingv1.NetworkPolicySpec{ PodSelector: metav1.LabelSelector{ @@ -176,6 +179,9 @@ func NetworkPolicyForIngress(req router.Request, resp router.Response) error { ObjectMeta: metav1.ObjectMeta{ Name: netPolName, Namespace: svc.Namespace, + Labels: map[string]string{ + labels.AcornManaged: "true", + }, }, Spec: networkingv1.NetworkPolicySpec{ PodSelector: metav1.LabelSelector{ @@ -258,6 +264,9 @@ func NetworkPolicyForService(req router.Request, resp router.Response) error { ObjectMeta: metav1.ObjectMeta{ Name: name.SafeConcatName(projectName, appName, service.Name, containerName), Namespace: service.Namespace, + Labels: map[string]string{ + labels.AcornManaged: "true", + }, }, Spec: networkingv1.NetworkPolicySpec{ PodSelector: metav1.LabelSelector{ From 4a356aedc96b7a47e795fcbd833fde642e0139ca Mon Sep 17 00:00:00 2001 From: Grant Linville Date: Fri, 31 Mar 2023 21:44:05 -0400 Subject: [PATCH 5/7] Use GC instead of a finalizer Signed-off-by: Grant Linville --- pkg/controller/routes.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/routes.go b/pkg/controller/routes.go index 87857b577..d87facadf 100644 --- a/pkg/controller/routes.go +++ b/pkg/controller/routes.go @@ -88,7 +88,7 @@ func routes(router *router.Router, registryTransport http.RoundTripper) { router.Type(&storagev1.StorageClass{}).HandlerFunc(volume.SyncVolumeClasses) router.Type(&corev1.Service{}).Selector(managedSelector).HandlerFunc(appdefinition.NetworkPolicyForService) router.Type(&netv1.Ingress{}).Selector(managedSelector).HandlerFunc(appdefinition.NetworkPolicyForIngress) - router.Type(&netv1.Ingress{}).Selector(managedSelector).FinalizeFunc("acorn.io/netpol", appdefinition.NetworkPolicyForIngress) + router.Type(&netv1.NetworkPolicy{}).Selector(managedSelector).HandlerFunc(gc.GCOrphans) configRouter := router.Type(&corev1.ConfigMap{}).Namespace(system.Namespace).Name(system.ConfigName) configRouter.Handler(config.NewDNSConfigHandler()) From d81e433cfb6051276a17091e3d58736e535721ef Mon Sep 17 00:00:00 2001 From: Grant Linville Date: Fri, 31 Mar 2023 21:51:18 -0400 Subject: [PATCH 6/7] Fix NetPol tests Signed-off-by: Grant Linville --- .../testdata/networkpolicy/appinstance/expected.yaml | 2 ++ .../testdata/networkpolicy/externalname/expected.yaml | 2 ++ .../testdata/networkpolicy/ingress/expected.yaml | 4 ++++ .../testdata/networkpolicy/service/expected.yaml | 2 ++ 4 files changed, 10 insertions(+) diff --git a/pkg/controller/appdefinition/testdata/networkpolicy/appinstance/expected.yaml b/pkg/controller/appdefinition/testdata/networkpolicy/appinstance/expected.yaml index ecbfff931..f6f0250ba 100644 --- a/pkg/controller/appdefinition/testdata/networkpolicy/appinstance/expected.yaml +++ b/pkg/controller/appdefinition/testdata/networkpolicy/appinstance/expected.yaml @@ -4,6 +4,8 @@ kind: NetworkPolicy metadata: name: app-name namespace: app-created-namespace + labels: + "acorn.io/managed": "true" spec: ingress: - from: diff --git a/pkg/controller/appdefinition/testdata/networkpolicy/externalname/expected.yaml b/pkg/controller/appdefinition/testdata/networkpolicy/externalname/expected.yaml index 345e9575b..a84a90580 100644 --- a/pkg/controller/appdefinition/testdata/networkpolicy/externalname/expected.yaml +++ b/pkg/controller/appdefinition/testdata/networkpolicy/externalname/expected.yaml @@ -3,6 +3,8 @@ kind: NetworkPolicy metadata: name: acorn-my-app-service-7777-service-7777-9999 namespace: my-app-namespace + labels: + "acorn.io/managed": "true" spec: ingress: - from: diff --git a/pkg/controller/appdefinition/testdata/networkpolicy/ingress/expected.yaml b/pkg/controller/appdefinition/testdata/networkpolicy/ingress/expected.yaml index 506a08355..dd573b544 100644 --- a/pkg/controller/appdefinition/testdata/networkpolicy/ingress/expected.yaml +++ b/pkg/controller/appdefinition/testdata/networkpolicy/ingress/expected.yaml @@ -4,6 +4,8 @@ kind: NetworkPolicy metadata: name: acorn-my-app-my-service-service-7777-9999-10000 namespace: my-app-namespace + labels: + "acorn.io/managed": "true" spec: ingress: - from: @@ -33,6 +35,8 @@ kind: NetworkPolicy metadata: name: acorn-my-app-my-service-nginx-9090-9090 namespace: my-app-namespace + labels: + "acorn.io/managed": "true" spec: ingress: - from: diff --git a/pkg/controller/appdefinition/testdata/networkpolicy/service/expected.yaml b/pkg/controller/appdefinition/testdata/networkpolicy/service/expected.yaml index c320faca8..2570d9b76 100644 --- a/pkg/controller/appdefinition/testdata/networkpolicy/service/expected.yaml +++ b/pkg/controller/appdefinition/testdata/networkpolicy/service/expected.yaml @@ -4,6 +4,8 @@ kind: NetworkPolicy metadata: name: acorn-my-app-one-publish-one namespace: my-app-namespace + labels: + "acorn.io/managed": "true" spec: podSelector: matchLabels: From b707aaeea54f987272635573c074e968c65a0915 Mon Sep 17 00:00:00 2001 From: Grant Linville Date: Sat, 1 Apr 2023 14:01:22 -0400 Subject: [PATCH 7/7] Remove finalizer check from NetworkPolicyForIngress Signed-off-by: Grant Linville --- pkg/controller/appdefinition/networkpolicy.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkg/controller/appdefinition/networkpolicy.go b/pkg/controller/appdefinition/networkpolicy.go index 64b9d90f3..168cecc26 100644 --- a/pkg/controller/appdefinition/networkpolicy.go +++ b/pkg/controller/appdefinition/networkpolicy.go @@ -76,12 +76,6 @@ func NetworkPolicyForApp(req router.Request, resp router.Response) error { // Acorn apps from the ingress controller. If the ingress controller namespace is not defined, traffic from // all namespaces will be allowed instead. func NetworkPolicyForIngress(req router.Request, resp router.Response) error { - // check if this is being called as a finalizer for a deleted Ingress - // we need this because we sometimes create NetworkPolicies in different namespaces from where their owning Ingresses live - if !req.Object.GetDeletionTimestamp().IsZero() { - return nil - } - cfg, err := config.Get(req.Ctx, req.Client) if err != nil { return err