diff --git a/pkg/app/piped/platformprovider/ecs/client.go b/pkg/app/piped/platformprovider/ecs/client.go index e466991e1a..0e4b6c1d1c 100644 --- a/pkg/app/piped/platformprovider/ecs/client.go +++ b/pkg/app/piped/platformprovider/ecs/client.go @@ -438,48 +438,58 @@ func (c *client) ModifyListeners(ctx context.Context, listenerArns []string, rou } 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), }) 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) } - // 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)), + }, }, }, - }, + } + modifiedActions = append(modifiedActions, modifiedAction) + } else { + modifiedActions = append(modifiedActions, action) } - 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) + } + } } } return nil diff --git a/pkg/app/piped/platformprovider/ecs/routing_traffic.go b/pkg/app/piped/platformprovider/ecs/routing_traffic.go index 4c7a94c647..df89fc99c3 100644 --- a/pkg/app/piped/platformprovider/ecs/routing_traffic.go +++ b/pkg/app/piped/platformprovider/ecs/routing_traffic.go @@ -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 +} diff --git a/pkg/app/piped/platformprovider/ecs/routing_traffic_test.go b/pkg/app/piped/platformprovider/ecs/routing_traffic_test.go new file mode 100644 index 0000000000..ce87da6824 --- /dev/null +++ b/pkg/app/piped/platformprovider/ecs/routing_traffic_test.go @@ -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:::targetgroup/xxx/yyy1", + Weight: 100, + }, + { + TargetGroupArn: "arn:aws:elasticloadbalancing:::targetgroup/xxx/yyy2", + Weight: 0, + }, + }, + actionTargets: []types.TargetGroupTuple{ + { + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:::targetgroup/xxx/yyy1"), + Weight: aws.Int32(100), + }, + { + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:::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:::targetgroup/xxx/yyy1", + Weight: 100, + }, + { + TargetGroupArn: "arn:aws:elasticloadbalancing:::targetgroup/xxx/yyy2", + Weight: 0, + }, + }, + actionTargets: []types.TargetGroupTuple{ + { + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:::targetgroup/xxx/yyy2"), + Weight: aws.Int32(0), + }, + { + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:::targetgroup/xxx/yyy1"), + Weight: aws.Int32(100), + }, + }, + expected: true, + }, + { + name: "the number of target groups are different", + cfg: RoutingTrafficConfig{ + { + TargetGroupArn: "arn:aws:elasticloadbalancing:::targetgroup/xxx/yyy1", + Weight: 100, + }, + }, + actionTargets: []types.TargetGroupTuple{ + { + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:::targetgroup/xxx/yyy1"), + Weight: aws.Int32(0), + }, + { + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:::targetgroup/xxx/yyy2"), + Weight: aws.Int32(100), + }, + }, + expected: false, + }, + { + name: "has a different target group", + cfg: RoutingTrafficConfig{ + { + TargetGroupArn: "arn:aws:elasticloadbalancing:::targetgroup/xxx/yyy1", + Weight: 100, + }, + { + TargetGroupArn: "arn:aws:elasticloadbalancing:::targetgroup/xxx/yyy2", + Weight: 0, + }, + }, + actionTargets: []types.TargetGroupTuple{ + { + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:::targetgroup/xxx/yyy1"), + Weight: aws.Int32(0), + }, + { + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:::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) + }) + } +}