Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Rohit-0505 committed Jul 21, 2023
1 parent 4458b3b commit 8031fcd
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 18 deletions.
2 changes: 1 addition & 1 deletion pkg/controller/bastion/actuator_delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ var _ = Describe("Bastion Host Delete", func() {
UserData: []byte("my-user"),
Ingress: []extensionsv1alpha1.BastionIngressPolicy{
{IPBlock: networkingv1.IPBlock{
CIDR: "213.69.151.0/24",
CIDR: "10.0.0.0/24",
}},
}},
}
Expand Down
21 changes: 7 additions & 14 deletions pkg/controller/bastion/actuator_reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package bastion

import (
"context"
"errors"
"fmt"
"net"
"net/netip"
Expand Down Expand Up @@ -48,6 +47,8 @@ import (
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
Expand Down Expand Up @@ -91,13 +92,10 @@ func (a *actuator) reconcile(ctx context.Context, log logr.Logger, bastion *exte
return fmt.Errorf("failed to create machine: %w", err)
}

err = ensureNetworkPolicy(ctx, namespace, bastion, onmetalClient, infraStatus, machine)
if err != nil {
if err = ensureNetworkPolicy(ctx, namespace, bastion, onmetalClient, infraStatus, machine); err != nil {
return fmt.Errorf("failed to create network policy: %w", err)
}

// TODO: NetworkPolicy to be added for shoot worker nodes

endpoints, err := getMachineEndpoints(machine)
if err != nil {
return fmt.Errorf("failed to get machine endpoints: %w", err)
Expand Down Expand Up @@ -251,7 +249,7 @@ func generateMachine(namespace string, bastionConfig *controllerconfig.BastionCo
NetworkInterfaceTemplate: &networkingv1alpha1.NetworkInterfaceTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"bastion-host": BastionInstanceName,
name: BastionInstanceName,
},
},
Spec: networkingv1alpha1.NetworkInterfaceSpec{
Expand Down Expand Up @@ -346,7 +344,7 @@ func ensureNetworkPolicy(ctx context.Context, namespace string, bastion *extensi
},
NetworkInterfaceSelector: metav1.LabelSelector{
MatchLabels: map[string]string{
"bastion-host": bastionHost.Name,
name: bastionHost.Name,
},
},
Ingress: []networkingv1alpha1.NetworkPolicyIngressRule{},
Expand Down Expand Up @@ -389,17 +387,12 @@ func getBastionIngressCIDR(bastion *extensionsv1alpha1.Bastion) ([]string, error
var cidrs []string
for _, ingress := range bastion.Spec.Ingress {
cidr := ingress.IPBlock.CIDR
ip, ipNet, err := net.ParseCIDR(cidr)
_, ipNet, err := net.ParseCIDR(cidr)
if err != nil {
return nil, fmt.Errorf("invalid ingress CIDR %q: %w", cidr, err)
}
normalisedCIDR := ipNet.String()

if ip.To4() != nil {
cidrs = append(cidrs, normalisedCIDR)
} else if ip.To16() != nil {
return nil, errors.New("IPv6 is currently not fully supported")
}
cidrs = append(cidrs, normalisedCIDR)
}
return cidrs, nil
}
6 changes: 3 additions & 3 deletions pkg/controller/bastion/actuator_reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,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 All @@ -74,6 +74,7 @@ var _ = Describe("Bastion Host Reconcile", func() {

By("ensuring bastion host is created with correct spec")
bastionHostName, err := generateBastionHostResourceName(cluster.ObjectMeta.Name, bastion)
Expect(err).ShouldNot(HaveOccurred())
bastionHost := &computev1alpha1.Machine{
ObjectMeta: metav1.ObjectMeta{
Namespace: ns.Name,
Expand Down Expand Up @@ -120,7 +121,6 @@ var _ = Describe("Bastion Host Reconcile", func() {
))

By("ensuring bastion network policy is created with correct spec")
Expect(err).ShouldNot(HaveOccurred())
networkPolicy := &networkingv1alpha1.NetworkPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: bastionHost.Name,
Expand All @@ -136,7 +136,7 @@ var _ = Describe("Bastion Host Reconcile", func() {
HaveField("Port", int32(sshPort)),
))),
HaveField("From", ContainElement(SatisfyAll(
HaveField("IPBlock.CIDR", commonv1alpha1.MustParseIPPrefix("213.69.151.0/24")),
HaveField("IPBlock.CIDR", commonv1alpha1.MustParseIPPrefix("10.0.0.0/24")),
))),
))),
))
Expand Down

0 comments on commit 8031fcd

Please sign in to comment.