Skip to content

Commit

Permalink
Add bastion host network policy implementation (#235)
Browse files Browse the repository at this point in the history
* Add bastion host network policy implementation

* address review comments
  • Loading branch information
Rohit-0505 authored Jul 24, 2023
1 parent 1d128dc commit 4063f5b
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 7 deletions.
25 changes: 23 additions & 2 deletions pkg/controller/bastion/actuator_delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down
90 changes: 88 additions & 2 deletions pkg/controller/bastion/actuator_reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package bastion
import (
"context"
"fmt"
"net"
"net/netip"
"time"

Expand All @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
26 changes: 24 additions & 2 deletions pkg/controller/bastion/actuator_reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
}},
},
},
Expand Down Expand Up @@ -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,
},
}
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion pkg/controller/bastion/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 4063f5b

Please sign in to comment.