From 4063f5b789f27370da7930cbf525396b7e89277e Mon Sep 17 00:00:00 2001 From: Rohit <53218775+Rohit-0505@users.noreply.github.com> Date: Mon, 24 Jul 2023 14:45:37 +0530 Subject: [PATCH] Add bastion host network policy implementation (#235) * Add bastion host network policy implementation * address review comments --- .../bastion/actuator_delete_test.go | 25 +++++- pkg/controller/bastion/actuator_reconcile.go | 90 ++++++++++++++++++- .../bastion/actuator_reconcile_test.go | 26 +++++- pkg/controller/bastion/options.go | 1 - 4 files changed, 135 insertions(+), 7 deletions(-) diff --git a/pkg/controller/bastion/actuator_delete_test.go b/pkg/controller/bastion/actuator_delete_test.go index 2aafd727..c03b325f 100644 --- a/pkg/controller/bastion/actuator_delete_test.go +++ b/pkg/controller/bastion/actuator_delete_test.go @@ -22,9 +22,11 @@ import ( extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" commonv1alpha1 "github.com/onmetal/onmetal-api/api/common/v1alpha1" computev1alpha1 "github.com/onmetal/onmetal-api/api/compute/v1alpha1" + networkingv1alpha1 "github.com/onmetal/onmetal-api/api/networking/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -53,8 +55,11 @@ var _ = Describe("Bastion Host Delete", func() { ProviderConfig: nil, }, UserData: []byte("my-user"), - Ingress: []extensionsv1alpha1.BastionIngressPolicy{}, - }, + Ingress: []extensionsv1alpha1.BastionIngressPolicy{ + {IPBlock: networkingv1.IPBlock{ + CIDR: "10.0.0.0/24", + }}, + }}, } Expect(k8sClient.Create(ctx, bastion)).Should(Succeed()) Eventually(Object(bastion)).Should(SatisfyAll( @@ -88,6 +93,22 @@ var _ = Describe("Bastion Host Delete", func() { HaveField("Data", HaveLen(1)), )) + By("ensuring network policy is created and owned by bastion host machine") + networkPolicy := &networkingv1alpha1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: bastionHostName, + Namespace: ns.Name, + }, + } + Eventually(Object(networkPolicy)).Should(SatisfyAll( + HaveField("Spec.NetworkRef.Name", "my-network"), + HaveField("ObjectMeta.OwnerReferences", ContainElement(SatisfyAll( + HaveField("Name", bastionHost.Name), + HaveField("Kind", "Machine"), + HaveField("UID", bastionHost.UID), + ))), + )) + By("patching bastion host with Running state") bastionHostBase := bastionHost.DeepCopy() bastionHost.Status.State = computev1alpha1.MachineStateRunning diff --git a/pkg/controller/bastion/actuator_reconcile.go b/pkg/controller/bastion/actuator_reconcile.go index caef0faf..04c5713c 100644 --- a/pkg/controller/bastion/actuator_reconcile.go +++ b/pkg/controller/bastion/actuator_reconcile.go @@ -17,6 +17,7 @@ package bastion import ( "context" "fmt" + "net" "net/netip" "time" @@ -31,6 +32,7 @@ import ( networkingv1alpha1 "github.com/onmetal/onmetal-api/api/networking/v1alpha1" storagev1alpha1 "github.com/onmetal/onmetal-api/api/storage/v1alpha1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -42,6 +44,13 @@ import ( "github.com/onmetal/gardener-extension-provider-onmetal/pkg/onmetal" ) +const ( + // sshPort is the default SSH Port used for bastion ingress firewall rule + sshPort = 22 + // name is the network interface label key + name = "bastion-host" +) + // bastionEndpoints collects the endpoints the bastion host provides; the // private endpoint is important for opening a port on the worker node // ingress network policy rule to allow SSH from that node, the public endpoint is where @@ -78,13 +87,15 @@ func (a *actuator) reconcile(ctx context.Context, log logr.Logger, bastion *exte return fmt.Errorf("failed to get onmetal client and namespace from cloudprovider secret: %w", err) } - // TODO: Add NetworkPolicy related implementation - machine, err := a.applyMachineAndIgnitionSecret(ctx, namespace, onmetalClient, infraStatus, opt) if err != nil { return fmt.Errorf("failed to create machine: %w", err) } + if err = ensureNetworkPolicy(ctx, namespace, bastion, onmetalClient, infraStatus, machine); err != nil { + return fmt.Errorf("failed to create network policy: %w", err) + } + endpoints, err := getMachineEndpoints(machine) if err != nil { return fmt.Errorf("failed to get machine endpoints: %w", err) @@ -236,6 +247,11 @@ func generateMachine(namespace string, bastionConfig *controllerconfig.BastionCo NetworkInterfaceSource: computev1alpha1.NetworkInterfaceSource{ Ephemeral: &computev1alpha1.EphemeralNetworkInterfaceSource{ NetworkInterfaceTemplate: &networkingv1alpha1.NetworkInterfaceTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + name: BastionInstanceName, + }, + }, Spec: networkingv1alpha1.NetworkInterfaceSpec{ NetworkRef: corev1.LocalObjectReference{ Name: infraStatus.NetworkRef.Name, @@ -310,3 +326,73 @@ func (be *bastionEndpoints) Ready() bool { func IngressReady(ingress *corev1.LoadBalancerIngress) bool { return ingress != nil && (ingress.Hostname != "" || ingress.IP != "") } + +func ensureNetworkPolicy(ctx context.Context, namespace string, bastion *extensionsv1alpha1.Bastion, onmetalClient client.Client, infraStatus *api.InfrastructureStatus, bastionHost *computev1alpha1.Machine) error { + cidrs, err := getBastionIngressCIDR(bastion) + if err != nil { + return fmt.Errorf("failed to get CIDR from bastion ingress: %w", err) + } + + networkPolicy := &networkingv1alpha1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: bastionHost.Name, + Namespace: namespace, + }, + Spec: networkingv1alpha1.NetworkPolicySpec{ + NetworkRef: corev1.LocalObjectReference{ + Name: infraStatus.NetworkRef.Name, + }, + NetworkInterfaceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + name: bastionHost.Name, + }, + }, + Ingress: []networkingv1alpha1.NetworkPolicyIngressRule{}, + PolicyTypes: []networkingv1alpha1.PolicyType{ + networkingv1alpha1.PolicyTypeIngress, + }, + }, + } + + for _, cidr := range cidrs { + ingressRule := networkingv1alpha1.NetworkPolicyIngressRule{ + Ports: []networkingv1alpha1.NetworkPolicyPort{ + { + Port: sshPort, + }, + }, + From: []networkingv1alpha1.NetworkPolicyPeer{ + { + IPBlock: &networkingv1alpha1.IPBlock{ + CIDR: commonv1alpha1.MustParseIPPrefix(cidr), + }, + }, + }, + } + networkPolicy.Spec.Ingress = append(networkPolicy.Spec.Ingress, ingressRule) + } + + if err := controllerutil.SetOwnerReference(bastionHost, networkPolicy, onmetalClient.Scheme()); err != nil { + return fmt.Errorf("failed to set owner reference for network policy %s: %w", client.ObjectKeyFromObject(networkPolicy), err) + } + + if _, err = controllerutil.CreateOrPatch(ctx, onmetalClient, networkPolicy, nil); err != nil { + return fmt.Errorf("failed to create or patch network policy %s: %w", client.ObjectKeyFromObject(networkPolicy), err) + } + + return err +} + +func getBastionIngressCIDR(bastion *extensionsv1alpha1.Bastion) ([]string, error) { + var cidrs []string + for _, ingress := range bastion.Spec.Ingress { + cidr := ingress.IPBlock.CIDR + _, ipNet, err := net.ParseCIDR(cidr) + if err != nil { + return nil, fmt.Errorf("invalid ingress CIDR %q: %w", cidr, err) + } + normalisedCIDR := ipNet.String() + cidrs = append(cidrs, normalisedCIDR) + } + return cidrs, nil +} diff --git a/pkg/controller/bastion/actuator_reconcile_test.go b/pkg/controller/bastion/actuator_reconcile_test.go index 123be963..2f8a4521 100644 --- a/pkg/controller/bastion/actuator_reconcile_test.go +++ b/pkg/controller/bastion/actuator_reconcile_test.go @@ -18,6 +18,7 @@ import ( "net/netip" extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller" + gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" commonv1alpha1 "github.com/onmetal/onmetal-api/api/common/v1alpha1" @@ -59,7 +60,7 @@ var _ = Describe("Bastion Host Reconcile", func() { UserData: []byte("abcd"), Ingress: []extensionsv1alpha1.BastionIngressPolicy{ {IPBlock: networkingv1.IPBlock{ - CIDR: "213.69.151.0/24", + CIDR: "10.0.0.0/24", }}, }, }, @@ -106,7 +107,7 @@ var _ = Describe("Bastion Host Reconcile", func() { By("ensuring ignition secret is created and owned by bastion host machine") ignitionSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: getIgnitionNameForMachine(bastionHostName), + Name: getIgnitionNameForMachine(bastionHost.Name), Namespace: ns.Name, }, } @@ -119,6 +120,27 @@ var _ = Describe("Bastion Host Reconcile", func() { HaveField("Data", HaveLen(1)), )) + By("ensuring bastion network policy is created with correct spec") + networkPolicy := &networkingv1alpha1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: bastionHost.Name, + Namespace: ns.Name, + }, + } + Eventually(Object(networkPolicy)).Should(SatisfyAll( + HaveField("Spec.NetworkRef.Name", "my-network"), + HaveField("Spec.NetworkInterfaceSelector.MatchLabels", HaveKeyWithValue("bastion-host", bastionHost.Name)), + HaveField("Spec.PolicyTypes", ContainElement(networkingv1alpha1.PolicyTypeIngress)), + HaveField("Spec.Ingress", ContainElement(SatisfyAll( + HaveField("Ports", ContainElement(SatisfyAll( + HaveField("Port", int32(sshPort)), + ))), + HaveField("From", ContainElement(SatisfyAll( + HaveField("IPBlock.CIDR", commonv1alpha1.MustParseIPPrefix("10.0.0.0/24")), + ))), + ))), + )) + By("patching bastion host with running state and network interfaces with private and virtual ip") machineBase := bastionHost.DeepCopy() bastionHost.Status.State = computev1alpha1.MachineStateRunning diff --git a/pkg/controller/bastion/options.go b/pkg/controller/bastion/options.go index 8c11781a..04cc810b 100644 --- a/pkg/controller/bastion/options.go +++ b/pkg/controller/bastion/options.go @@ -26,7 +26,6 @@ import ( type Options struct { BastionInstanceName string UserData []byte - //TODO: add networkPolicy } // DetermineOptions determines the required information that are required to reconcile a Bastion on onmetal. This