Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

Fix bring-your-own-alb for plugin/ecs #4457

Merged
merged 13 commits into from
Feb 1, 2023
Merged

Fix bring-your-own-alb for plugin/ecs #4457

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
cd2d5d5
Allow bringing your own alb listener
izaaklauer Jan 25, 2023
82374c2
changelog
izaaklauer Jan 25, 2023
a011631
docs
izaaklauer Jan 25, 2023
8f6f0e5
First pass at removing the alb listener as input
izaaklauer Jan 26, 2023
5254429
Allowing bring-your-own-alb for ecs
izaaklauer Jan 27, 2023
13444be
Changelog and docs
izaaklauer Jan 27, 2023
ec027df
Always setting listener config, as we manage the listener.
izaaklauer Jan 27, 2023
dec90fd
Allowing successful "releases" when deployments don't use an alb.
izaaklauer Jan 27, 2023
6470259
Fixing changelog number
izaaklauer Jan 27, 2023
c090257
Update builtin/aws/ecs/platform.go
izaaklauer Jan 27, 2023
42da790
Removing listener_arn field
izaaklauer Jan 27, 2023
d75b0b6
Fixing tests
izaaklauer Jan 27, 2023
a99fff4
Allow users to bring their own alb _and_ specify a cert
izaaklauer Jan 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/4457.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
plugin/aws-ecs: Fix bringing your own alb to ecs deployments.
```
167 changes: 82 additions & 85 deletions builtin/aws/ecs/platform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,25 +66,25 @@ func (p *Platform) ConfigSet(config interface{}) error {
alb := c.ALB
err := utils.Error(validation.ValidateStruct(alb,
validation.Field(&alb.CertificateId,
validation.Empty.When(alb.ListenerARN != "").Error("certificate cannot be used with listener_arn"),
validation.Empty.When(alb.LoadBalancerArn != "").Error("certificate cannot be used with load_balancer_arn"),
),
validation.Field(&alb.ZoneId,
validation.Empty.When(alb.ListenerARN != ""),
validation.Empty.When(alb.LoadBalancerArn != ""),
validation.Required.When(alb.FQDN != ""),
),
validation.Field(&alb.FQDN,
validation.Empty.When(alb.ListenerARN != ""),
validation.Empty.When(alb.LoadBalancerArn != ""),
validation.Required.When(alb.ZoneId != "").Error("fqdn only valid with zone_id"),
),
validation.Field(&alb.InternalScheme,
validation.Nil.When(alb.ListenerARN != "").Error("internal cannot be used with listener_arn"),
validation.Nil.When(alb.LoadBalancerArn != "").Error("internal cannot be used with load_balancer_arn"),
),
validation.Field(&alb.ListenerARN,
validation.Field(&alb.LoadBalancerArn,
validation.Empty.When(
alb.CertificateId != "" ||
alb.ZoneId != "" ||
alb.FQDN != "" ||
len(alb.SecurityGroupIDs) >= 1).Error("listener_arn can not be used with other options"),
len(alb.SecurityGroupIDs) >= 1).Error("load_balancer_arn can not be used with other options"),
),
validation.Field(&alb.SecurityGroupIDs),
))
Expand Down Expand Up @@ -397,6 +397,9 @@ func (p *Platform) Deploy(
albState := rm.Resource("application load balancer").State().(*Resource_Alb)
result.LoadBalancerArn = albState.Arn

listenerState := rm.Resource("alb listener").State().(*Resource_Alb_Listener)
result.ListenerArn = listenerState.Arn

cState := rm.Resource("cluster").State().(*Resource_Cluster)
result.Cluster = cState.Name

Expand Down Expand Up @@ -1000,6 +1003,8 @@ func (p *Platform) resourceServiceDestroy(
return nil
}

// resourceAlbListenerCreate finds or creates an ALB listener, and ensures that the
// target group is added to it.
func (p *Platform) resourceAlbListenerCreate(
ctx context.Context,
sg terminal.StepGroup,
Expand All @@ -1016,7 +1021,7 @@ func (p *Platform) resourceAlbListenerCreate(
return nil
}

s := sg.Add("Initiating ALB creation")
s := sg.Add("Initiating ALB Listener")
defer s.Abort()

state.TargetGroup = targetGroup
Expand All @@ -1031,9 +1036,8 @@ func (p *Platform) resourceAlbListenerCreate(
}

var (
certs []*elbv2.Certificate
protocol = "HTTP"
newListener = false
certs []*elbv2.Certificate
protocol = "HTTP"
)

if albConfig != nil && albConfig.CertificateId != "" {
Expand All @@ -1045,75 +1049,31 @@ func (p *Platform) resourceAlbListenerCreate(

elbsrv := elbv2.New(sess)

var listener *elbv2.Listener

if albConfig != nil && albConfig.ListenerARN != "" {
s.Update("Describing requested ALB listener (ARN: %s)", albConfig.ListenerARN)

state.Managed = false

out, err := elbsrv.DescribeListenersWithContext(ctx, &elbv2.DescribeListenersInput{
ListenerArns: []*string{aws.String(albConfig.ListenerARN)},
})
if err != nil {
return status.Errorf(codes.Internal, "failed to describe requested listener ARN %q: %s", albConfig.ListenerARN, err)
}

listener = out.Listeners[0]
s.Update("Using configured ALB Listener: %s (load-balancer: %s)",
*listener.ListenerArn, *listener.LoadBalancerArn)
} else {
state.Managed = true

if alb == nil || alb.Arn == "" {
return status.Errorf(codes.InvalidArgument, "cannot create ALB listener - no existing ALB defined.")
}

s.Update("No ALB listener specified - looking for listeners for ALB %q", alb.Name)
listeners, err := elbsrv.DescribeListenersWithContext(ctx, &elbv2.DescribeListenersInput{
LoadBalancerArn: &alb.Arn,
})
if err != nil {
return status.Errorf(codes.Internal, "failed to describe listeners for alb (ARN %q): %s", alb.Arn, err)
}

if len(listeners.Listeners) > 0 {
listener = listeners.Listeners[0]
s.Update("Using existing ALB Listener (ARN: %q)", *listener.ListenerArn)
} else {
s.Update("Creating new ALB Listener")
newListener = true

log.Info("load-balancer defined", "dns-name", alb.DnsName)

tgs[0].Weight = aws.Int64(100)
lo, err := elbsrv.CreateListenerWithContext(ctx, &elbv2.CreateListenerInput{
LoadBalancerArn: &alb.Arn,
Port: aws.Int64(int64(externalIngressPort)),
Protocol: aws.String(protocol),
Certificates: certs,
DefaultActions: []*elbv2.Action{
{
ForwardConfig: &elbv2.ForwardActionConfig{
TargetGroups: tgs,
},
Type: aws.String("forward"),
},
},
})
if err != nil {
return status.Errorf(codes.Internal, "failed to create listener: %s", err)
}
if alb == nil || alb.Arn == "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ I see that this if block already existed, but how is it enforced that the ALB is created before the listener is created? In the resource manager I can see that the ALB listener is sequentially after the ALB itself - is that what ensures that it is done first?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argmapper magic! This resourceAlbListenerCreate function takes an *Resource_Alb as input, and goes into the argmapper soup along with the resourceAlbCreate function that provides it. Argmapper figures out the right order to invoke the whole string of resource functions.

return status.Errorf(codes.InvalidArgument, "cannot create ALB listener - no existing ALB defined.")
}
log.Info("load-balancer defined", "dns-name", alb.DnsName)

listener = lo.Listeners[0]
s.Update("Looking for listeners for ALB %q", alb.Name)
listeners, err := elbsrv.DescribeListenersWithContext(ctx, &elbv2.DescribeListenersInput{
LoadBalancerArn: &alb.Arn,
})
if err != nil {
return status.Errorf(codes.Internal, "failed to describe listeners for alb (ARN %q): %s", alb.Arn, err)
}

s.Update("Created ALB Listener")
log.Debug("Created ALB Listener", "arn", *listener.ListenerArn)
// Check for existing listeners on our specified port
var listener *elbv2.Listener
for _, l := range listeners.Listeners {
if *l.Port == int64(externalIngressPort) {
listener = l
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, we were always plucking the first listener from the ALB. Now, we make sure to modify the one that corresponds to our port of interest.

This is important if someone brings an ALB that already has a listener on another port that's unrelated to this waypoint app.

}
state.Arn = *listener.ListenerArn

if !newListener {
if listener != nil {
// Found an existing listener!
s.Update("Modifying existing ALB Listener to introduce target group")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was no UI output previously! Now it reports progress to users.


def := listener.DefaultActions

if len(def) > 0 && def[0].ForwardConfig != nil {
Expand All @@ -1125,13 +1085,40 @@ func (p *Platform) resourceAlbListenerCreate(
}
}

s.Update("Modifying ALB Listener to introduce target group")
in := &elbv2.ModifyListenerInput{
ListenerArn: listener.ListenerArn,
DefaultActions: []*elbv2.Action{
{
ForwardConfig: &elbv2.ForwardActionConfig{
TargetGroups: tgs,
},
Type: aws.String("forward"),
},
},

_, err := elbsrv.ModifyListenerWithContext(ctx, &elbv2.ModifyListenerInput{
ListenerArn: listener.ListenerArn,
// Setting these on every deployment so that we pick up any potential changes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

// in the waypoint.hcl config
Port: aws.Int64(int64(externalIngressPort)),
Protocol: aws.String(protocol),
Certificates: certs,
}

_, err = elbsrv.ModifyListenerWithContext(ctx, in)
if err != nil {
return status.Errorf(codes.Internal, "failed to introduce new target group to existing ALB listener: %s", err)
}

s.Update("Modified ALB Listener to introduce target group")
} else {
s.Update("Creating new ALB Listener")
state.Managed = true

tgs[0].Weight = aws.Int64(100)
lo, err := elbsrv.CreateListenerWithContext(ctx, &elbv2.CreateListenerInput{
LoadBalancerArn: &alb.Arn,
Port: aws.Int64(int64(externalIngressPort)),
Protocol: aws.String(protocol),
Certificates: certs,
DefaultActions: []*elbv2.Action{
{
ForwardConfig: &elbv2.ForwardActionConfig{
Expand All @@ -1142,12 +1129,17 @@ func (p *Platform) resourceAlbListenerCreate(
},
})
if err != nil {
return status.Errorf(codes.Internal, "failed to introduce new target group to existing ALB listener: %s", err)
return status.Errorf(codes.Internal, "failed to create listener: %s", err)
}

s.Update("Modified ALB Listener to introduce target group")
listener = lo.Listeners[0]

s.Update("Created ALB Listener")
log.Debug("Created ALB Listener", "arn", *listener.ListenerArn)
}

state.Arn = *listener.ListenerArn

s.Done()
return nil
}
Expand Down Expand Up @@ -1720,7 +1712,6 @@ func (p *Platform) resourceAlbCreate(
}
state.Arn = *lb.LoadBalancerArn

state.Arn = *lb.LoadBalancerArn
state.DnsName = *lb.DNSName
state.CanonicalHostedZoneId = *lb.CanonicalHostedZoneId

Expand Down Expand Up @@ -2627,6 +2618,10 @@ type ALBConfig struct {
// ALB Listener ARN. This allows for usage of existing ALBs.
ListenerARN string `hcl:"listener_arn,optional"`

// When set, waypoint will configure the target group into the load balancer.
// This allows for usage of existing ALBs.
LoadBalancerArn string `hcl:"load_balancer_arn"`

// Indicates, when creating an ALB, that it should be internal rather than
// internet facing.
InternalScheme *bool `hcl:"internal,optional"`
Expand Down Expand Up @@ -2940,13 +2935,15 @@ deploy {
)

doc.SetField(
"listener_arn",
"load_balancer_arn",
"the ARN on an existing ALB to configure",
docs.Summary(
"when this is set, no ALB or Listener is created. Instead the application is",
"configured by manipulating this existing Listener. This allows users to",
"configure their ALB outside waypoint but still have waypoint hook the application",
"to that ALB",
"when this is set, Waypoint will use this ALB instead of creating",
"it's own. A target group will still be created for each deployment,",
izaaklauer marked this conversation as resolved.
Show resolved Hide resolved
"and will be added to a listener on the configured ALB port",
"(Waypoint will the listener if it doesn't exist).",
"This allows users to configure their ALB outside Waypoint but still ",
"have Waypoint hook the application to that ALB",
),
)

Expand Down
Loading