Merge pull request #14468 from johngmyers/bastion-nlb

Create NLB instead of CLB for bastion

cloudmock/aws/mockelbv2/api.go

@@ -19,6 +19,8 @@ package mockelbv2
 import (
 	"sync"
 
+	"k8s.io/kops/cloudmock/aws/mockec2"
+
 	"github.com/aws/aws-sdk-go/service/elbv2"
 	"github.com/aws/aws-sdk-go/service/elbv2/elbv2iface"
 )
@@ -28,6 +30,7 @@ type MockELBV2 struct {
 
 	mutex sync.Mutex
 
+	EC2           *mockec2.MockEC2
 	LoadBalancers map[string]*loadBalancer
 	lbCount       int
 	TargetGroups  map[string]*targetGroup

cloudmock/aws/mockelbv2/loadbalancers.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/aws/aws-sdk-go/service/elbv2"
 	"k8s.io/klog/v2"
 )
@@ -88,10 +89,17 @@ func (m *MockELBV2) CreateLoadBalancer(request *elbv2.CreateLoadBalancerInput) (
 		CanonicalHostedZoneId: aws.String("HZ123456"),
 	}
 	zones := make([]*elbv2.AvailabilityZone, 0)
+	vpc := "vpc-1"
 	for _, subnet := range request.Subnets {
 		zones = append(zones, &elbv2.AvailabilityZone{
 			SubnetId: subnet,
 		})
+		subnetsOutput, err := m.EC2.DescribeSubnets(&ec2.DescribeSubnetsInput{
+			SubnetIds: []*string{subnet},
+		})
+		if err == nil {
+			vpc = *subnetsOutput.Subnets[0].VpcId
+		}
 	}
 	for _, subnetMapping := range request.SubnetMappings {
 		var lbAddrs []*elbv2.LoadBalancerAddress
@@ -105,12 +113,16 @@ func (m *MockELBV2) CreateLoadBalancer(request *elbv2.CreateLoadBalancerInput) (
 			SubnetId:              subnetMapping.SubnetId,
 			LoadBalancerAddresses: lbAddrs,
 		})
+		subnetsOutput, err := m.EC2.DescribeSubnets(&ec2.DescribeSubnetsInput{
+			SubnetIds: []*string{subnetMapping.SubnetId},
+		})
+		if err == nil {
+			vpc = *subnetsOutput.Subnets[0].VpcId
+		}
 	}
 	lb.AvailabilityZones = zones
 
-	// This is hardcoded because AWS derives it from the subnets above
-	// But we don'y rely on the NLB's VPC ID at all in awstasks
-	lb.VpcId = aws.String("vpc-1")
+	lb.VpcId = aws.String(vpc)
 
 	m.lbCount++
 	arn := fmt.Sprintf("arn:aws-test:elasticloadbalancing:us-test-1:000000000000:loadbalancer/net/%v/%v", aws.StringValue(request.Name), m.lbCount)

docs/releases/1.26-NOTES.md

@@ -6,7 +6,11 @@ This is a document to gather the release notes prior to the release.
 
 # Significant changes
 
-* Instance group images can now be dynamically fetched through an AWS SSM Parameter (AWS only).
+## AWS only
+
+* Bastions are now fronted by a Network Load Balancer.
+
+* Instance group images can now be dynamically fetched through an AWS SSM Parameter.
 
 
 # Breaking changes

k8s/crds/kops.k8s.io_clusters.yaml

@@ -5604,13 +5604,13 @@ spec:
                       bastionPublicName:
                         type: string
                       idleTimeoutSeconds:
-                        description: IdleTimeoutSeconds is the bastion's Loadbalancer
-                          idle timeout
+                        description: IdleTimeoutSeconds is unused
                         format: int64
                         type: integer
                       loadBalancer:
                         properties:
                           additionalSecurityGroups:
+                            description: AdditionalSecurityGroups is unused
                             items:
                               type: string
                             type: array

pkg/apis/kops/bastion.go

@@ -19,14 +19,11 @@ package kops
 type BastionSpec struct {
 	// PublicName is the domain name for the bastion load balancer.
 	PublicName string `json:"publicName,omitempty"`
-	// IdleTimeoutSeconds is the bastion's load balancer idle timeout.
-	IdleTimeoutSeconds *int64 `json:"idleTimeoutSeconds,omitempty"`
 	// LoadBalancer contains settings for the load balancer fronting bastion instances.
 	LoadBalancer *BastionLoadBalancerSpec `json:"loadBalancer,omitempty"`
 }
 
 type BastionLoadBalancerSpec struct {
-	AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
 	// Type of load balancer to create, it can be Public or Internal.
 	Type LoadBalancerType `json:"type,omitempty"`
 }

pkg/apis/kops/v1alpha2/bastion.go

@@ -18,12 +18,15 @@ package v1alpha2
 
 type BastionSpec struct {
 	PublicName string `json:"bastionPublicName,omitempty"`
-	// IdleTimeoutSeconds is the bastion's Loadbalancer idle timeout
+	// IdleTimeoutSeconds is unused
+	// +k8s:conversion-gen=false
 	IdleTimeoutSeconds *int64                   `json:"idleTimeoutSeconds,omitempty"`
 	LoadBalancer       *BastionLoadBalancerSpec `json:"loadBalancer,omitempty"`
 }
 
 type BastionLoadBalancerSpec struct {
+	// AdditionalSecurityGroups is unused
+	// +k8s:conversion-gen=false
 	AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
 	// Type of load balancer to create, it can be Public or Internal.
 	Type LoadBalancerType `json:"type,omitempty"`

pkg/apis/kops/v1alpha3/bastion.go

@@ -19,14 +19,11 @@ package v1alpha3
 type BastionSpec struct {
 	// PublicName is the domain name for the bastion load balancer.
 	PublicName string `json:"publicName,omitempty"`
-	// IdleTimeoutSeconds is the bastion's load balancer idle timeout.
-	IdleTimeoutSeconds *int64 `json:"idleTimeoutSeconds,omitempty"`
 	// LoadBalancer contains settings for the load balancer fronting bastion instances.
 	LoadBalancer *BastionLoadBalancerSpec `json:"loadBalancer,omitempty"`
 }
 
 type BastionLoadBalancerSpec struct {
-	AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
 	// Type of load balancer to create, it can be Public or Internal.
 	Type LoadBalancerType `json:"type,omitempty"`
 }

pkg/apis/kops/validation/validation.go

@@ -414,16 +414,9 @@ func validateTopology(c *kops.Cluster, topology *kops.TopologySpec, fieldPath *f
 	}
 
 	if topology.Bastion != nil {
-		bastion := topology.Bastion
 		if topology.Masters == kops.TopologyPublic || topology.Nodes == kops.TopologyPublic {
 			allErrs = append(allErrs, field.Forbidden(fieldPath.Child("bastion"), "bastion requires masters and nodes to have private topology"))
 		}
-		if bastion.IdleTimeoutSeconds != nil && *bastion.IdleTimeoutSeconds <= 0 {
-			allErrs = append(allErrs, field.Invalid(fieldPath.Child("bastion", "idleTimeoutSeconds"), *bastion.IdleTimeoutSeconds, "bastion idleTimeoutSeconds should be greater than zero"))
-		}
-		if bastion.IdleTimeoutSeconds != nil && *bastion.IdleTimeoutSeconds > 3600 {
-			allErrs = append(allErrs, field.Invalid(fieldPath.Child("bastion", "idleTimeoutSeconds"), *bastion.IdleTimeoutSeconds, "bastion idleTimeoutSeconds cannot be greater than one hour"))
-		}
 	}
 
 	if topology.DNS != nil {

pkg/model/awsmodel/autoscalinggroup.go

@@ -460,7 +460,7 @@ func (b *AutoscalingGroupModelBuilder) buildAutoScalingGroupTask(c *fi.ModelBuil
 		}
 
 		if ig.Spec.Role == kops.InstanceGroupRoleBastion {
-			t.LoadBalancers = append(t.LoadBalancers, b.LinkToCLB("bastion"))
+			t.TargetGroups = append(t.TargetGroups, b.LinkToTargetGroup("bastion"))
 		}
 	}