Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ECS] Modify ELB listener rules other than defaults without adding config #4733

Merged
merged 9 commits into from
Dec 27, 2023
76 changes: 43 additions & 33 deletions pkg/app/piped/platformprovider/ecs/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,48 +438,58 @@
}

for _, listenerArn := range listenerArns {
// Describe the listener to get the current actions
describeListenersOutput, err := c.elbClient.DescribeListeners(ctx, &elasticloadbalancingv2.DescribeListenersInput{
ListenerArns: []string{listenerArn},
describeRulesOutput, err := c.elbClient.DescribeRules(ctx, &elasticloadbalancingv2.DescribeRulesInput{
ListenerArn: aws.String(listenerArn),

Check warning on line 442 in pkg/app/piped/platformprovider/ecs/client.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/ecs/client.go#L441-L442

Added lines #L441 - L442 were not covered by tests
})
if err != nil {
return fmt.Errorf("error describing listener %s: %w", listenerArn, err)
return fmt.Errorf("failed to describe rules of listener %s: %w", listenerArn, err)

Check warning on line 445 in pkg/app/piped/platformprovider/ecs/client.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/ecs/client.go#L445

Added line #L445 was not covered by tests
}

// Prepare the actions to be modified
var modifiedActions []elbtypes.Action
for _, action := range describeListenersOutput.Listeners[0].DefaultActions {
if action.Type == elbtypes.ActionTypeEnumForward {
// Modify only the forward action
modifiedAction := elbtypes.Action{
Type: elbtypes.ActionTypeEnumForward,
ForwardConfig: &elbtypes.ForwardActionConfig{
TargetGroups: []elbtypes.TargetGroupTuple{
{
TargetGroupArn: aws.String(routingTrafficCfg[0].TargetGroupArn),
Weight: aws.Int32(int32(routingTrafficCfg[0].Weight)),
},
{
TargetGroupArn: aws.String(routingTrafficCfg[1].TargetGroupArn),
Weight: aws.Int32(int32(routingTrafficCfg[1].Weight)),
for _, rule := range describeRulesOutput.Rules {
modifiedActions := make([]elbtypes.Action, 0, len(rule.Actions))
for _, action := range rule.Actions {
if action.Type == elbtypes.ActionTypeEnumForward && routingTrafficCfg.hasSameTargets(action.ForwardConfig.TargetGroups) {
// Modify only the forward action which has the same target groups.
modifiedAction := elbtypes.Action{
Type: elbtypes.ActionTypeEnumForward,
Order: action.Order,
ForwardConfig: &elbtypes.ForwardActionConfig{
TargetGroups: []elbtypes.TargetGroupTuple{
{
TargetGroupArn: aws.String(routingTrafficCfg[0].TargetGroupArn),
Weight: aws.Int32(int32(routingTrafficCfg[0].Weight)),
},
{
TargetGroupArn: aws.String(routingTrafficCfg[1].TargetGroupArn),
Weight: aws.Int32(int32(routingTrafficCfg[1].Weight)),
},

Check warning on line 465 in pkg/app/piped/platformprovider/ecs/client.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/ecs/client.go#L448-L465

Added lines #L448 - L465 were not covered by tests
},
},
},
}
modifiedActions = append(modifiedActions, modifiedAction)
} else {
modifiedActions = append(modifiedActions, action)

Check warning on line 471 in pkg/app/piped/platformprovider/ecs/client.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/ecs/client.go#L468-L471

Added lines #L468 - L471 were not covered by tests
}
modifiedActions = append(modifiedActions, modifiedAction)
} else {
// Keep other actions unchanged
modifiedActions = append(modifiedActions, action)
}
}

// Modify the listener
_, err = c.elbClient.ModifyListener(ctx, &elasticloadbalancingv2.ModifyListenerInput{
ListenerArn: aws.String(listenerArn),
DefaultActions: modifiedActions,
})
if err != nil {
return fmt.Errorf("error modifying listener %s: %w", listenerArn, err)
// The default rule needs to be modified by ModifyListener API.
if rule.IsDefault {
_, err := c.elbClient.ModifyListener(ctx, &elasticloadbalancingv2.ModifyListenerInput{
ListenerArn: &listenerArn,
DefaultActions: modifiedActions,
})
if err != nil {
return fmt.Errorf("failed to modify default rule %s: %w", *rule.RuleArn, err)
}
} else {
_, err := c.elbClient.ModifyRule(ctx, &elasticloadbalancingv2.ModifyRuleInput{
RuleArn: rule.RuleArn,
Actions: modifiedActions,
})
if err != nil {
return fmt.Errorf("failed to modify rule %s: %w", *rule.RuleArn, err)
}

Check warning on line 491 in pkg/app/piped/platformprovider/ecs/client.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/ecs/client.go#L476-L491

Added lines #L476 - L491 were not covered by tests
}
}
}
return nil
Expand Down
23 changes: 23 additions & 0 deletions pkg/app/piped/platformprovider/ecs/routing_traffic.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,32 @@

package ecs

import (
"github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
)

type RoutingTrafficConfig []targetGroupWeight

type targetGroupWeight struct {
TargetGroupArn string
Weight int
}

func (c RoutingTrafficConfig) hasSameTargets(forwardActionTargets []types.TargetGroupTuple) bool {
if len(c) != len(forwardActionTargets) {
return false
}

cMap := make(map[string]struct{})
for _, item := range c {
cMap[item.TargetGroupArn] = struct{}{}
}

for _, target := range forwardActionTargets {
if _, ok := cMap[*target.TargetGroupArn]; !ok {
return false
}
}

return true
}
135 changes: 135 additions & 0 deletions pkg/app/piped/platformprovider/ecs/routing_traffic_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
// Copyright 2023 The PipeCD Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package ecs

import (
"testing"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
"github.com/stretchr/testify/assert"
)

func TestHasSameTargets(t *testing.T) {
t.Parallel()

testcases := []struct {
name string
cfg RoutingTrafficConfig
actionTargets []types.TargetGroupTuple
expected bool
}{
{
name: "has the same target groups in the same order",
cfg: RoutingTrafficConfig{
{
TargetGroupArn: "arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy1",
Weight: 100,
},
{
TargetGroupArn: "arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy2",
Weight: 0,
},
},
actionTargets: []types.TargetGroupTuple{
{
TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy1"),
Weight: aws.Int32(100),
},
{
TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy2"),
Weight: aws.Int32(0),
},
},
expected: true,
},
{
name: "has the same target groups in the different order",
cfg: RoutingTrafficConfig{
{
TargetGroupArn: "arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy1",
Weight: 100,
},
{
TargetGroupArn: "arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy2",
Weight: 0,
},
},
actionTargets: []types.TargetGroupTuple{
{
TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy2"),
Weight: aws.Int32(0),
},
{
TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy1"),
Weight: aws.Int32(100),
},
},
expected: true,
},
{
name: "the number of target groups are different",
cfg: RoutingTrafficConfig{
{
TargetGroupArn: "arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy1",
Weight: 100,
},
},
actionTargets: []types.TargetGroupTuple{
{
TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy1"),
Weight: aws.Int32(0),
},
{
TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy2"),
Weight: aws.Int32(100),
},
},
expected: false,
},
{
name: "has a different target group",
cfg: RoutingTrafficConfig{
{
TargetGroupArn: "arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy1",
Weight: 100,
},
{
TargetGroupArn: "arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy2",
Weight: 0,
},
},
actionTargets: []types.TargetGroupTuple{
{
TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy1"),
Weight: aws.Int32(0),
},
{
TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:<region>:<account-id>:targetgroup/xxx/yyy3"),
Weight: aws.Int32(100),
},
},
expected: false,
},
}

for _, tc := range testcases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
hasSame := tc.cfg.hasSameTargets(tc.actionTargets)
assert.Equal(t, tc.expected, hasSame)
})
}
}
Loading