Merge pull request #10276 from hakman/fix-asg

Parse TargetGroup names from ARNs
This commit is contained in:
Kubernetes Prow Robot 2020-11-21 12:21:33 -08:00 committed by GitHub
commit 44465075b3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 117 additions and 21 deletions

View File

@ -130,6 +130,12 @@ func ValidateInstanceGroup(g *kops.InstanceGroup, cloud fi.Cloud) field.ErrorLis
allErrs = append(allErrs, awsValidateInstanceGroup(g, cloud.(awsup.AWSCloud))...)
}
for i, lb := range g.Spec.ExternalLoadBalancers {
path := field.NewPath("spec", "externalLoadBalancers").Index(i)
allErrs = append(allErrs, validateExternalLoadBalancer(&lb, path)...)
}
return allErrs
}
@ -278,3 +284,51 @@ func validateCloudLabels(ig *kops.InstanceGroup, fldPath *field.Path) (allErrs f
}
return allErrs
}
func validateExternalLoadBalancer(lb *kops.LoadBalancer, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if lb.LoadBalancerName != nil && lb.TargetGroupARN != nil {
allErrs = append(allErrs, field.TooMany(fldPath, 2, 1))
}
if lb.LoadBalancerName != nil {
name := fi.StringValue(lb.LoadBalancerName)
if len(name) > 32 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("loadBalancerName"), name,
"Load Balancer name must have at most 32 characters"))
}
}
if lb.TargetGroupARN != nil {
actual := fi.StringValue(lb.TargetGroupARN)
parsed, err := arn.Parse(actual)
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath.Child("targetGroupArn"), actual,
fmt.Sprintf("Target Group ARN must be a valid AWS ARN: %v", err)))
return allErrs
}
resource := strings.Split(parsed.Resource, "/")
if len(resource) != 3 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("targetGroupArn"), actual,
"Target Group ARN resource must be a valid AWS ARN resource such as \"targetgroup/tg-name/1234567890123456\""))
return allErrs
}
kind := resource[0]
if kind != "targetgroup" {
allErrs = append(allErrs, field.Invalid(fldPath.Child("targetGroupArn"), kind,
"Target Group ARN resource type must be \"targetgroup\""))
}
name := resource[1]
if len(name) > 32 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("targetGroupArn"), name,
"Target Group ARN resource name must have at most 32 characters"))
}
}
return allErrs
}

View File

@ -21,6 +21,7 @@ go_library(
"//upup/pkg/fi:go_default_library",
"//upup/pkg/fi/cloudup/awstasks:go_default_library",
"//upup/pkg/fi/fitasks:go_default_library",
"//vendor/github.com/aws/aws-sdk-go/aws/arn:go_default_library",
"//vendor/github.com/aws/aws-sdk-go/service/ec2:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/klog/v2:go_default_library",

View File

@ -18,8 +18,11 @@ package awsmodel
import (
"fmt"
"sort"
"strings"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/service/ec2"
"k8s.io/klog/v2"
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/featureflag"
@ -28,8 +31,6 @@ import (
"k8s.io/kops/pkg/model/spotinstmodel"
"k8s.io/kops/upup/pkg/fi"
"k8s.io/kops/upup/pkg/fi/cloudup/awstasks"
"github.com/aws/aws-sdk-go/service/ec2"
)
const (
@ -388,7 +389,7 @@ func (b *AutoscalingGroupModelBuilder) buildAutoScalingGroupTask(c *fi.ModelBuil
}
}
for i, extLB := range ig.Spec.ExternalLoadBalancers {
for _, extLB := range ig.Spec.ExternalLoadBalancers {
if extLB.LoadBalancerName != nil {
lb := &awstasks.ClassicLoadBalancer{
Name: extLB.LoadBalancerName,
@ -400,8 +401,16 @@ func (b *AutoscalingGroupModelBuilder) buildAutoScalingGroupTask(c *fi.ModelBuil
}
if extLB.TargetGroupARN != nil {
parsed, err := arn.Parse(fi.StringValue(extLB.TargetGroupARN))
if err != nil {
return nil, fmt.Errorf("error parsing target grup ARN: %v", err)
}
resource := strings.Split(parsed.Resource, "/")
if len(resource) != 3 || resource[0] != "targetgroup" {
return nil, fmt.Errorf("error parsing target grup ARN resource: %q", parsed.Resource)
}
tg := &awstasks.TargetGroup{
Name: fi.String(fmt.Sprintf("external-tg-%d", i)),
Name: fi.String(resource[1]),
ARN: extLB.TargetGroupARN,
Shared: fi.Bool(true),
}
@ -409,6 +418,7 @@ func (b *AutoscalingGroupModelBuilder) buildAutoScalingGroupTask(c *fi.ModelBuil
c.AddTask(tg)
}
}
sort.Stable(awstasks.OrderTargetGroupsByName(t.TargetGroups))
// @step: are we using a mixed instance policy
if ig.Spec.MixedInstancesPolicy != nil {

View File

@ -247,8 +247,15 @@ func (h *IntegrationTestHarness) SetupMockAWS() *awsup.MockAWSCloud {
}, "nat-b2345678")
mockELBV2.CreateTargetGroup(&elbv2.CreateTargetGroupInput{
Name: aws.String("my-external-tg"),
Name: aws.String("my-external-tg-1"),
})
mockELBV2.CreateTargetGroup(&elbv2.CreateTargetGroupInput{
Name: aws.String("my-external-tg-2"),
})
mockELBV2.CreateTargetGroup(&elbv2.CreateTargetGroupInput{
Name: aws.String("my-external-tg-3"),
})
return cloud
}

View File

@ -75,10 +75,12 @@
}
],
"LoadBalancerNames": [
"my-other-elb"
"my-external-elb-2",
"my-external-elb-3"
],
"TargetGroupARNs": [
"arn:aws:elasticloadbalancing:us-test-1:000000000000:targetgroup/my-external-tg/1"
"arn:aws:elasticloadbalancing:us-test-1:000000000000:targetgroup/my-external-tg-2/2",
"arn:aws:elasticloadbalancing:us-test-1:000000000000:targetgroup/my-external-tg-3/3"
]
}
},
@ -157,7 +159,10 @@
}
],
"LoadBalancerNames": [
"my-elb"
"my-external-elb-1"
],
"TargetGroupARNs": [
"arn:aws:elasticloadbalancing:us-test-1:000000000000:targetgroup/my-external-tg-1/1"
]
}
},

View File

@ -58,7 +58,8 @@ spec:
subnets:
- us-test-1a
externalLoadBalancers:
- loadBalancerName: my-elb
- targetGroupArn: arn:aws:elasticloadbalancing:us-test-1:000000000000:targetgroup/my-external-tg-1/1
- loadBalancerName: my-external-elb-1
---
@ -79,7 +80,9 @@ spec:
subnets:
- us-test-1a
externalLoadBalancers:
- targetGroupArn: arn:aws:elasticloadbalancing:us-test-1:000000000000:targetgroup/my-external-tg/1
- loadBalancerName: my-other-elb
- targetGroupArn: arn:aws:elasticloadbalancing:us-test-1:000000000000:targetgroup/my-external-tg-2/2
- targetGroupArn: arn:aws:elasticloadbalancing:us-test-1:000000000000:targetgroup/my-external-tg-3/3
- loadBalancerName: my-external-elb-2
- loadBalancerName: my-external-elb-3

View File

@ -86,7 +86,7 @@ resource "aws_autoscaling_group" "master-us-test-1a-masters-externallb-example-c
id = aws_launch_template.master-us-test-1a-masters-externallb-example-com.id
version = aws_launch_template.master-us-test-1a-masters-externallb-example-com.latest_version
}
load_balancers = ["my-other-elb"]
load_balancers = ["my-external-elb-2", "my-external-elb-3"]
max_size = 1
metrics_granularity = "1Minute"
min_size = 1
@ -126,7 +126,7 @@ resource "aws_autoscaling_group" "master-us-test-1a-masters-externallb-example-c
propagate_at_launch = true
value = "owned"
}
target_group_arns = ["arn:aws:elasticloadbalancing:us-test-1:000000000000:targetgroup/my-external-tg/1"]
target_group_arns = ["arn:aws:elasticloadbalancing:us-test-1:000000000000:targetgroup/my-external-tg-2/2", "arn:aws:elasticloadbalancing:us-test-1:000000000000:targetgroup/my-external-tg-3/3"]
vpc_zone_identifier = [aws_subnet.us-test-1a-externallb-example-com.id]
}
@ -136,7 +136,7 @@ resource "aws_autoscaling_group" "nodes-externallb-example-com" {
id = aws_launch_template.nodes-externallb-example-com.id
version = aws_launch_template.nodes-externallb-example-com.latest_version
}
load_balancers = ["my-elb"]
load_balancers = ["my-external-elb-1"]
max_size = 2
metrics_granularity = "1Minute"
min_size = 2
@ -176,6 +176,7 @@ resource "aws_autoscaling_group" "nodes-externallb-example-com" {
propagate_at_launch = true
value = "owned"
}
target_group_arns = ["arn:aws:elasticloadbalancing:us-test-1:000000000000:targetgroup/my-external-tg-1/1"]
vpc_zone_identifier = [aws_subnet.us-test-1a-externallb-example-com.id]
}

View File

@ -90,6 +90,7 @@ go_library(
"//util/pkg/maps:go_default_library",
"//util/pkg/slice:go_default_library",
"//vendor/github.com/aws/aws-sdk-go/aws:go_default_library",
"//vendor/github.com/aws/aws-sdk-go/aws/arn:go_default_library",
"//vendor/github.com/aws/aws-sdk-go/aws/awserr:go_default_library",
"//vendor/github.com/aws/aws-sdk-go/service/autoscaling:go_default_library",
"//vendor/github.com/aws/aws-sdk-go/service/ec2:go_default_library",

View File

@ -23,6 +23,7 @@ import (
"strings"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/service/autoscaling"
"k8s.io/klog/v2"
"k8s.io/kops/upup/pkg/fi"
@ -163,13 +164,22 @@ func (e *AutoscalingGroup) Find(c *fi.Context) (*AutoscalingGroup, error) {
if len(g.TargetGroupARNs) > 0 {
actualTGs := make([]*TargetGroup, 0)
for _, tg := range g.TargetGroupARNs {
actualTGs = append(actualTGs, &TargetGroup{ARN: aws.String(*tg)})
parsed, err := arn.Parse(fi.StringValue(tg))
if err != nil {
return nil, fmt.Errorf("error parsing target grup ARN: %v", err)
}
resource := strings.Split(parsed.Resource, "/")
if len(resource) != 3 || resource[0] != "targetgroup" {
return nil, fmt.Errorf("error parsing target grup ARN resource: %q", parsed.Resource)
}
actualTGs = append(actualTGs, &TargetGroup{ARN: aws.String(*tg), Name: aws.String(resource[1])})
}
targetGroups, err := ReconcileTargetGroups(c.Cloud.(awsup.AWSCloud), actualTGs, e.TargetGroups)
if err != nil {
return nil, err
}
actual.TargetGroups = targetGroups
sort.Stable(OrderTargetGroupsByName(actual.TargetGroups))
}
if g.VPCZoneIdentifier != nil {
@ -523,9 +533,11 @@ func (v *AutoscalingGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Autos
var attachLBRequest *autoscaling.AttachLoadBalancersInput
var detachLBRequest *autoscaling.DetachLoadBalancersInput
if changes.LoadBalancers != nil {
attachLBRequest = &autoscaling.AttachLoadBalancersInput{
AutoScalingGroupName: e.Name,
LoadBalancerNames: e.AutoscalingLoadBalancers(),
if e != nil && len(e.LoadBalancers) > 0 {
attachLBRequest = &autoscaling.AttachLoadBalancersInput{
AutoScalingGroupName: e.Name,
LoadBalancerNames: e.AutoscalingLoadBalancers(),
}
}
if a != nil && len(a.LoadBalancers) > 0 {
@ -539,9 +551,11 @@ func (v *AutoscalingGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Autos
var attachTGRequest *autoscaling.AttachLoadBalancerTargetGroupsInput
var detachTGRequest *autoscaling.DetachLoadBalancerTargetGroupsInput
if changes.TargetGroups != nil {
attachTGRequest = &autoscaling.AttachLoadBalancerTargetGroupsInput{
AutoScalingGroupName: e.Name,
TargetGroupARNs: e.AutoscalingTargetGroups(),
if e != nil && len(e.TargetGroups) > 0 {
attachTGRequest = &autoscaling.AttachLoadBalancerTargetGroupsInput{
AutoScalingGroupName: e.Name,
TargetGroupARNs: e.AutoscalingTargetGroups(),
}
}
if a != nil && len(a.TargetGroups) > 0 {