From 9e78352d515f0813990dfe990bfe44b57048ee77 Mon Sep 17 00:00:00 2001 From: moko-poi Date: Sun, 12 Nov 2023 00:21:59 +0900 Subject: [PATCH 01/11] feat(ECS): enable selection of listener rules Signed-off-by: moko-poi --- pkg/app/piped/executor/ecs/ecs.go | 8 +- pkg/app/piped/executor/ecs/rollback.go | 10 +- pkg/app/piped/platformprovider/ecs/client.go | 154 ++++++++++++++++--- pkg/app/piped/platformprovider/ecs/ecs.go | 3 +- 4 files changed, 149 insertions(+), 26 deletions(-) diff --git a/pkg/app/piped/executor/ecs/ecs.go b/pkg/app/piped/executor/ecs/ecs.go index ee025b5b21..2c03450868 100644 --- a/pkg/app/piped/executor/ecs/ecs.go +++ b/pkg/app/piped/executor/ecs/ecs.go @@ -451,7 +451,13 @@ func routing(ctx context.Context, in *executor.Input, platformProviderName strin return false } - if err := client.ModifyListeners(ctx, currListenerArns, routingTrafficCfg); err != nil { + currListenerRuleArns, err := client.GetListenerRuleArns(ctx, currListenerArns) + if err != nil { + in.LogPersister.Errorf("Failed to get current active listener rule: %v", err) + return false + } + + if err := client.ModifyListenerOrRule(ctx, currListenerArns, currListenerRuleArns, routingTrafficCfg); err != nil { in.LogPersister.Errorf("Failed to routing traffic to PRIMARY/CANARY variants: %v", err) return false } diff --git a/pkg/app/piped/executor/ecs/rollback.go b/pkg/app/piped/executor/ecs/rollback.go index 87c76306ff..7a28211e12 100644 --- a/pkg/app/piped/executor/ecs/rollback.go +++ b/pkg/app/piped/executor/ecs/rollback.go @@ -158,8 +158,14 @@ func rollback(ctx context.Context, in *executor.Input, platformProviderName stri return false } - if err := client.ModifyListeners(ctx, currListenerArns, routingTrafficCfg); err != nil { - in.LogPersister.Errorf("Failed to routing traffic to PRIMARY variant: %v", err) + currListenerRuleArns, err := client.GetListenerRuleArns(ctx, currListenerArns) + if err != nil { + in.LogPersister.Errorf("Failed to get current active listener rule: %v", err) + return false + } + + if err := client.ModifyListenerOrRule(ctx, currListenerArns, currListenerRuleArns, routingTrafficCfg); err != nil { + in.LogPersister.Errorf("Failed to routing traffic to PRIMARY/CANARY variants: %v", err) return false } } diff --git a/pkg/app/piped/platformprovider/ecs/client.go b/pkg/app/piped/platformprovider/ecs/client.go index 3b16819f80..b7ba84fbbb 100644 --- a/pkg/app/piped/platformprovider/ecs/client.go +++ b/pkg/app/piped/platformprovider/ecs/client.go @@ -417,6 +417,30 @@ func (c *client) GetListenerArns(ctx context.Context, targetGroup types.LoadBala return arns, nil } +func (c *client) GetListenerRuleArns(ctx context.Context, listenerArns []string) ([]string, error) { + var ruleArns []string + + // 各リスナーのルールを取得 + for _, listenerArn := range listenerArns { + input := &elasticloadbalancingv2.DescribeRulesInput{ + ListenerArn: aws.String(listenerArn), + } + output, err := c.elbClient.DescribeRules(ctx, input) + if err != nil { + return nil, err + } + for _, rule := range output.Rules { + ruleArns = append(ruleArns, *rule.RuleArn) + } + } + + if len(ruleArns) == 0 { + return nil, platformprovider.ErrNotFound + } + + return ruleArns, nil +} + func (c *client) getLoadBalancerArn(ctx context.Context, targetGroupArn string) (string, error) { input := &elasticloadbalancingv2.DescribeTargetGroupsInput{ TargetGroupArns: []string{targetGroupArn}, @@ -432,41 +456,127 @@ func (c *client) getLoadBalancerArn(ctx context.Context, targetGroupArn string) return output.TargetGroups[0].LoadBalancerArns[0], nil } -func (c *client) ModifyListeners(ctx context.Context, listenerArns []string, routingTrafficCfg RoutingTrafficConfig) error { +func (c *client) ModifyListenerOrRule(ctx context.Context, listenerArns []string, listenerRuleArns []string, routingTrafficCfg RoutingTrafficConfig) error { if len(routingTrafficCfg) != 2 { return fmt.Errorf("invalid listener configuration: requires 2 target groups") } - modifyListener := func(ctx context.Context, listenerArn string) error { - input := &elasticloadbalancingv2.ModifyListenerInput{ - ListenerArn: aws.String(listenerArn), - DefaultActions: []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)), + if len(listenerRuleArns) > 0 { + modifyListenerRule := func(ctx context.Context, listenerRuleArn string) error { + input := &elasticloadbalancingv2.ModifyRuleInput{ + RuleArn: aws.String(listenerRuleArn), + Actions: []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)), + }, }, }, }, }, - }, + } + _, err := c.elbClient.ModifyRule(ctx, input) + return err } - _, err := c.elbClient.ModifyListener(ctx, input) - return err - } - for _, listener := range listenerArns { - if err := modifyListener(ctx, listener); err != nil { + for _, ruleArn := range listenerRuleArns { + // Describe the rule to get current actions + describeRulesOutput, err := c.elbClient.DescribeRules(ctx, &elasticloadbalancingv2.DescribeRulesInput{ + RuleArns: []string{ruleArn}, + }) + if err != nil { + return fmt.Errorf("error describing listener rule %v: %w", ruleArn, err) + } + + // No rules found, nothing to modify + if len(describeRulesOutput.Rules) == 0 { + return fmt.Errorf("no rules found for ARN: %s", ruleArn) + } + + // Check if the current action type is forward + for _, rule := range describeRulesOutput.Rules { + for _, action := range rule.Actions { + if action.Type == elbtypes.ActionTypeEnumForward { + // Call modifyListenerRule only if action type is forward + if err := modifyListenerRule(ctx, ruleArn); err != nil { + return err + } + // Break the loop once the modify function is called for the rule + break + } + } + } + } + + return nil + } + + if len(listenerRuleArns) == 0 { + modifyListener := func(ctx context.Context, listenerArn string) error { + input := &elasticloadbalancingv2.ModifyListenerInput{ + ListenerArn: aws.String(listenerArn), + DefaultActions: []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)), + }, + }, + }, + }, + }, + } + _, err := c.elbClient.ModifyListener(ctx, input) return err } + + for _, listenerArn := range listenerArns { + // Describe the listener to get current default actions + describeListenerOutput, err := c.elbClient.DescribeListeners(ctx, &elasticloadbalancingv2.DescribeListenersInput{ + ListenerArns: []string{listenerArn}, + }) + if err != nil { + return fmt.Errorf("error describing listener %v: %w", listenerArn, err) + } + + // No listeners found, nothing to modify + if len(describeListenerOutput.Listeners) == 0 { + return fmt.Errorf("no listeners found for ARN: %s", listenerArn) + } + + // Check if the current default action type is forward + for _, listener := range describeListenerOutput.Listeners { + for _, action := range listener.DefaultActions { + if action.Type == elbtypes.ActionTypeEnumForward { + // Call modifyListener only if default action type is forward + if err := modifyListener(ctx, listenerArn); err != nil { + return err + } + // Break the loop once the modify function is called for the listener + break + } + } + } + } + + return nil } + return nil } diff --git a/pkg/app/piped/platformprovider/ecs/ecs.go b/pkg/app/piped/platformprovider/ecs/ecs.go index 692f50fb24..dda41a07e3 100644 --- a/pkg/app/piped/platformprovider/ecs/ecs.go +++ b/pkg/app/piped/platformprovider/ecs/ecs.go @@ -57,7 +57,8 @@ type ECS interface { type ELB interface { GetListenerArns(ctx context.Context, targetGroup types.LoadBalancer) ([]string, error) - ModifyListeners(ctx context.Context, listenerArns []string, routingTrafficCfg RoutingTrafficConfig) error + GetListenerRuleArns(ctx context.Context, listenerArns []string) ([]string, error) + ModifyListenerOrRule(ctx context.Context, listenerArns []string, listenerRuleArns []string, routingTrafficCfg RoutingTrafficConfig) error } // Registry holds a pool of aws client wrappers. From 41b67d663a5c0aacc5ea8c18350cb532842d6177 Mon Sep 17 00:00:00 2001 From: Khanh Tran <32532742+khanhtc1202@users.noreply.github.com> Date: Mon, 13 Nov 2023 16:49:42 +0900 Subject: [PATCH 02/11] Update pkg/app/piped/platformprovider/ecs/client.go Signed-off-by: moko-poi --- pkg/app/piped/platformprovider/ecs/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/app/piped/platformprovider/ecs/client.go b/pkg/app/piped/platformprovider/ecs/client.go index b7ba84fbbb..5d678d15b9 100644 --- a/pkg/app/piped/platformprovider/ecs/client.go +++ b/pkg/app/piped/platformprovider/ecs/client.go @@ -420,7 +420,7 @@ func (c *client) GetListenerArns(ctx context.Context, targetGroup types.LoadBala func (c *client) GetListenerRuleArns(ctx context.Context, listenerArns []string) ([]string, error) { var ruleArns []string - // 各リスナーのルールを取得 + // Fetch all rules by listeners for _, listenerArn := range listenerArns { input := &elasticloadbalancingv2.DescribeRulesInput{ ListenerArn: aws.String(listenerArn), From 78de40bfb931db8497c7b4e3a5c1e49680a51da1 Mon Sep 17 00:00:00 2001 From: Masafumi Ikeyama <54844746+mi11km@users.noreply.github.com> Date: Mon, 13 Nov 2023 14:41:54 +0900 Subject: [PATCH 03/11] Make actions plan preview handle timeout option (#4648) * Format and make tests pass in tool/actions-plan-preview Signed-off-by: mi11km * Use --piped-handle-timeout to make the timeout arg activate Signed-off-by: mi11km * Add tests to model/notificationevent (#4631) * Add tests to model/notificationevent Signed-off-by: Kenta Kozuka * Add Parallel() Signed-off-by: Kenta Kozuka --------- Signed-off-by: Kenta Kozuka Signed-off-by: mi11km * add piped-handle-timeout(input args) Signed-off-by: mi11km --------- Signed-off-by: mi11km Signed-off-by: Kenta Kozuka Co-authored-by: Kenta Kozuka Co-authored-by: Khanh Tran <32532742+khanhtc1202@users.noreply.github.com> Signed-off-by: moko-poi --- tool/actions-plan-preview/main.go | 32 +++-- tool/actions-plan-preview/main_test.go | 134 ++++++++++++++++++ tool/actions-plan-preview/planpreview.go | 2 + .../testdata/comment-has-failed-app.txt | 7 +- .../testdata/comment-has-failed-piped.txt | 7 +- .../testdata/comment-has-no-diff-apps.txt | 6 +- .../testdata/comment-no-env.txt | 5 +- .../testdata/comment-only-changed-app.txt | 5 +- 8 files changed, 178 insertions(+), 20 deletions(-) diff --git a/tool/actions-plan-preview/main.go b/tool/actions-plan-preview/main.go index bcb94c95fe..34f321eff6 100644 --- a/tool/actions-plan-preview/main.go +++ b/tool/actions-plan-preview/main.go @@ -36,11 +36,12 @@ const ( commentEventName = "issue_comment" pushEventName = "push" - argAddress = "address" - argAPIKey = "api-key" - argToken = "token" - argTimeout = "timeout" - argPRNum = "pull-request-number" + argAddress = "address" + argAPIKey = "api-key" + argToken = "token" + argTimeout = "timeout" + argPipedHandleTimeout = "piped-handle-timeout" + argPRNum = "pull-request-number" ) func main() { @@ -121,6 +122,7 @@ func main() { args.Address, args.APIKey, args.Timeout, + args.PipedHandleTimeout, ) if err != nil { doComment(failureBadgeURL + "\nUnable to run plan-preview. \ncause: " + err.Error()) @@ -172,11 +174,12 @@ func main() { } type arguments struct { - Address string - APIKey string - Token string - Timeout time.Duration - PRNum int + Address string + APIKey string + Token string + Timeout time.Duration + PipedHandleTimeout time.Duration + PRNum int } func parseArgs(args []string) (arguments, error) { @@ -200,6 +203,12 @@ func parseArgs(args []string) (arguments, error) { return arguments{}, err } out.Timeout = d + case argPipedHandleTimeout: + d, err := time.ParseDuration(ps[1]) + if err != nil { + return arguments{}, err + } + out.PipedHandleTimeout = d case argPRNum: if ps[1] == "" { continue @@ -227,6 +236,9 @@ func parseArgs(args []string) (arguments, error) { if out.Timeout == 0 { out.Timeout = defaultTimeout } + if out.PipedHandleTimeout == 0 { + out.PipedHandleTimeout = defaultTimeout + } return out, nil } diff --git a/tool/actions-plan-preview/main_test.go b/tool/actions-plan-preview/main_test.go index b098fed85d..14d96dc068 100644 --- a/tool/actions-plan-preview/main_test.go +++ b/tool/actions-plan-preview/main_test.go @@ -16,8 +16,11 @@ package main import ( "embed" + "fmt" "testing" + "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -33,3 +36,134 @@ func readTestdataFile(t *testing.T, name string) []byte { func boolPointer(b bool) *bool { return &b } + +func Test_parseArgs(t *testing.T) { + type args struct { + args []string + } + tests := []struct { + name string + args args + want arguments + wantErr assert.ErrorAssertionFunc + }{ + { + name: "minimum required args with no error", + args: args{ + args: []string{ + "address=localhost:8080", + "api-key=xxxxxxxxxxxxxx", + "token=xxxxxxxxxxxxxxxx", + }, + }, + want: arguments{ + Address: "localhost:8080", + APIKey: "xxxxxxxxxxxxxx", + Token: "xxxxxxxxxxxxxxxx", + Timeout: defaultTimeout, + PipedHandleTimeout: defaultTimeout, + }, + wantErr: assert.NoError, + }, + { + name: "minimum required args and specified timeout arg with no error", + args: args{ + args: []string{ + "address=localhost:8080", + "api-key=xxxxxxxxxxxxxx", + "token=xxxxxxxxxxxxxxxx", + "timeout=10m", + }, + }, + want: arguments{ + Address: "localhost:8080", + APIKey: "xxxxxxxxxxxxxx", + Token: "xxxxxxxxxxxxxxxx", + Timeout: 10 * time.Minute, + PipedHandleTimeout: defaultTimeout, + }, + wantErr: assert.NoError, + }, + { + name: "minimum required args and specified piped-handle-timeout arg with no error", + args: args{ + args: []string{ + "address=localhost:8080", + "api-key=xxxxxxxxxxxxxx", + "token=xxxxxxxxxxxxxxxx", + "piped-handle-timeout=10m", + }, + }, + want: arguments{ + Address: "localhost:8080", + APIKey: "xxxxxxxxxxxxxx", + Token: "xxxxxxxxxxxxxxxx", + Timeout: defaultTimeout, + PipedHandleTimeout: 10 * time.Minute, + }, + wantErr: assert.NoError, + }, + { + name: "minimum required args and specified timeout and piped-handle-timeout arg with no error", + args: args{ + args: []string{ + "address=localhost:8080", + "api-key=xxxxxxxxxxxxxx", + "token=xxxxxxxxxxxxxxxx", + "timeout=12m", + "piped-handle-timeout=15m", + }, + }, + want: arguments{ + Address: "localhost:8080", + APIKey: "xxxxxxxxxxxxxx", + Token: "xxxxxxxxxxxxxxxx", + Timeout: 12 * time.Minute, + PipedHandleTimeout: 15 * time.Minute, + }, + wantErr: assert.NoError, + }, + { + name: "missing required args (address) returns error", + args: args{ + args: []string{ + "api-key=xxxxxxxxxxxxxx", + "token=xxxxxxxxxxxxxxxx", + }, + }, + want: arguments{}, + wantErr: assert.Error, + }, + { + name: "missing required args (api-key) returns error", + args: args{ + args: []string{ + "address=localhost:8080", + "token=xxxxxxxxxxxxxxxx", + }, + }, + want: arguments{}, + wantErr: assert.Error, + }, + { + name: "missing required args (token) returns error", + args: args{ + args: []string{ + "address=localhost:8080", + "api-key=xxxxxxxxxxxxxx", + }, + }, + want: arguments{}, + wantErr: assert.Error, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseArgs(tt.args.args) + if tt.wantErr(t, err, fmt.Sprintf("parseArgs(%v)", tt.args.args)) { + return + } + assert.Equalf(t, tt.want, got, "parseArgs(%v)", tt.args.args) + }) + } +} diff --git a/tool/actions-plan-preview/planpreview.go b/tool/actions-plan-preview/planpreview.go index c30dbff490..48a7d2f3e3 100644 --- a/tool/actions-plan-preview/planpreview.go +++ b/tool/actions-plan-preview/planpreview.go @@ -84,6 +84,7 @@ func retrievePlanPreview( address, apiKey string, timeout time.Duration, + pipedHandleTimeout time.Duration, ) (*PlanPreviewResult, error) { dir, err := os.MkdirTemp("", "") @@ -101,6 +102,7 @@ func retrievePlanPreview( "--address", address, "--api-key", apiKey, "--timeout", timeout.String(), + "--piped-handle-timeout", pipedHandleTimeout.String(), "--out", outPath, } cmd := exec.CommandContext(ctx, "pipectl", args...) diff --git a/tool/actions-plan-preview/testdata/comment-has-failed-app.txt b/tool/actions-plan-preview/testdata/comment-has-failed-app.txt index 451a394c9d..34734d3a00 100644 --- a/tool/actions-plan-preview/testdata/comment-has-failed-app.txt +++ b/tool/actions-plan-preview/testdata/comment-has-failed-app.txt @@ -3,7 +3,9 @@ Ran plan-preview against head commit abc of this pull request. PipeCD detected `1` updated applications and here are their plan results. Once this pull request got merged their deployments will be triggered to run as these estimations. -## app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1 +## Plans + +### app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1 Sync strategy: PIPELINE Summary: plan-summary-1 @@ -17,12 +19,11 @@ plan-details-1

---- ## NOTE **An error occurred while building plan-preview for the following applications** -## app: [app-name-2](app-url-2), env: env-2, kind: app-kind-2 +### app: [app-name-2](app-url-2), env: env-2, kind: app-kind-2 Reason: reason-2 diff --git a/tool/actions-plan-preview/testdata/comment-has-failed-piped.txt b/tool/actions-plan-preview/testdata/comment-has-failed-piped.txt index 633ccca518..5075144422 100644 --- a/tool/actions-plan-preview/testdata/comment-has-failed-piped.txt +++ b/tool/actions-plan-preview/testdata/comment-has-failed-piped.txt @@ -3,7 +3,9 @@ Ran plan-preview against head commit abc of this pull request. PipeCD detected `1` updated applications and here are their plan results. Once this pull request got merged their deployments will be triggered to run as these estimations. -## app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1 +## Plans + +### app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1 Sync strategy: PIPELINE Summary: plan-summary-1 @@ -17,12 +19,11 @@ plan-details-1

---- ## NOTE **An error occurred while building plan-preview for applications of the following Pipeds** -## piped: [piped-id-1](piped-url-1) +### piped: [piped-id-1](piped-url-1) Reason: piped-reason-1 diff --git a/tool/actions-plan-preview/testdata/comment-has-no-diff-apps.txt b/tool/actions-plan-preview/testdata/comment-has-no-diff-apps.txt index 9b9f548857..bdb55d2781 100644 --- a/tool/actions-plan-preview/testdata/comment-has-no-diff-apps.txt +++ b/tool/actions-plan-preview/testdata/comment-has-no-diff-apps.txt @@ -3,7 +3,9 @@ Ran plan-preview against head commit abc of this pull request. PipeCD detected `3` updated applications and here are their plan results. Once this pull request got merged their deployments will be triggered to run as these estimations. -## app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1 +## Plans + +### app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1 Sync strategy: PIPELINE Summary: plan-summary-1 @@ -17,7 +19,7 @@ plan-details-1

-## No resource changes were detected but the following apps will also be triggered +### No resource changes were detected but the following apps will also be triggered ###### `PIPELINE` diff --git a/tool/actions-plan-preview/testdata/comment-no-env.txt b/tool/actions-plan-preview/testdata/comment-no-env.txt index 3a413ea0f9..7afcbf2d63 100644 --- a/tool/actions-plan-preview/testdata/comment-no-env.txt +++ b/tool/actions-plan-preview/testdata/comment-no-env.txt @@ -3,7 +3,9 @@ Ran plan-preview against head commit abc of this pull request. PipeCD detected `1` updated applications and here are their plan results. Once this pull request got merged their deployments will be triggered to run as these estimations. -## app: [app-name-1](app-url-1), kind: app-kind-1 +## Plans + +### app: [app-name-1](app-url-1), kind: app-kind-1 Sync strategy: PIPELINE Summary: plan-summary-1 @@ -16,3 +18,4 @@ plan-details-1 ```

+ diff --git a/tool/actions-plan-preview/testdata/comment-only-changed-app.txt b/tool/actions-plan-preview/testdata/comment-only-changed-app.txt index 7424801491..da1f4a5dc0 100644 --- a/tool/actions-plan-preview/testdata/comment-only-changed-app.txt +++ b/tool/actions-plan-preview/testdata/comment-only-changed-app.txt @@ -3,7 +3,9 @@ Ran plan-preview against head commit abc of this pull request. PipeCD detected `1` updated applications and here are their plan results. Once this pull request got merged their deployments will be triggered to run as these estimations. -## app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1 +## Plans + +### app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1 Sync strategy: PIPELINE Summary: plan-summary-1 @@ -16,3 +18,4 @@ plan-details-1 ```

+ From 5ea306a6e8eab6962f8899758e12e974404a2dd7 Mon Sep 17 00:00:00 2001 From: Kenta Kozuka Date: Tue, 14 Nov 2023 10:20:57 +0900 Subject: [PATCH 04/11] Increase limit of stale action operations (#4671) Signed-off-by: moko-poi --- .github/workflows/stale.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/stale.yaml b/.github/workflows/stale.yaml index 05adbd3322..5e59280f57 100644 --- a/.github/workflows/stale.yaml +++ b/.github/workflows/stale.yaml @@ -9,6 +9,7 @@ jobs: steps: - uses: actions/stale@v8 with: + operations-per-run: 1000 # Issues stale-issue-message: 'This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.' close-issue-message: 'This issue was closed because it has been stalled for 7 days with no activity. Feel free to reopen if still applicable.' From 5b3ac3f66cb425cdcad483236371953cfda01ccc Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane <40124947+ffjlabo@users.noreply.github.com> Date: Tue, 14 Nov 2023 18:10:58 +0900 Subject: [PATCH 05/11] Add docs for k8s annotations (#4673) Signed-off-by: Yoshiki Fujikane Signed-off-by: moko-poi --- .../en/docs-dev/user-guide/configuration-reference.md | 8 ++++++++ .../en/docs-v0.45.x/user-guide/configuration-reference.md | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/docs/content/en/docs-dev/user-guide/configuration-reference.md b/docs/content/en/docs-dev/user-guide/configuration-reference.md index 6f94c24137..436c7ecbc1 100644 --- a/docs/content/en/docs-dev/user-guide/configuration-reference.md +++ b/docs/content/en/docs-dev/user-guide/configuration-reference.md @@ -40,6 +40,14 @@ spec: | eventWatcher | [][EventWatcher](#eventwatcher) | List of configurations for event watcher. | No | | driftDetection | [DriftDetection](#driftdetection) | Configuration for drift detection. | No | +### Annotations + +Kubernetes resources can be managed by some annotations provided by PipeCD. + +| Annotation key | Target resource(es) | Possible values | Description | +| `pipecd.dev/ignore-drift-detection` | any | "true" | Whether the drift detection should ignore this resource. | +| `pipecd.dev/server-side-apply` | any | "true" | Use server side apply instead of client side apply. | + ## Terraform application ``` yaml diff --git a/docs/content/en/docs-v0.45.x/user-guide/configuration-reference.md b/docs/content/en/docs-v0.45.x/user-guide/configuration-reference.md index 2c04c5b50e..bd815274ac 100644 --- a/docs/content/en/docs-v0.45.x/user-guide/configuration-reference.md +++ b/docs/content/en/docs-v0.45.x/user-guide/configuration-reference.md @@ -40,6 +40,14 @@ spec: | eventWatcher | [][EventWatcher](#eventwatcher) | List of configurations for event watcher. | No | | driftDetection | [DriftDetection](#driftdetection) | Configuration for drift detection. | No | +### Annotations + +Kubernetes resources can be managed by some annotations provided by PipeCD. + +| Annotation key | Target resource(es) | Possible values | Description | +| `pipecd.dev/ignore-drift-detection` | any | "true" | Whether the drift detection should ignore this resource. | +| `pipecd.dev/server-side-apply` | any | "true" | Use server side apply instead of client side apply. | + ## Terraform application ``` yaml From 965ba41a05d1fa84ec306451fa078ef62b93c2bd Mon Sep 17 00:00:00 2001 From: moko-poi Date: Fri, 24 Nov 2023 10:06:27 +0900 Subject: [PATCH 06/11] feat: add modifylisner and modifyrule Signed-off-by: moko-poi --- pkg/app/piped/executor/ecs/ecs.go | 9 +- pkg/app/piped/executor/ecs/rollback.go | 9 +- pkg/app/piped/platformprovider/ecs/client.go | 166 ++++++++----------- pkg/app/piped/platformprovider/ecs/ecs.go | 3 +- 4 files changed, 89 insertions(+), 98 deletions(-) diff --git a/pkg/app/piped/executor/ecs/ecs.go b/pkg/app/piped/executor/ecs/ecs.go index 2c03450868..9c5a80f112 100644 --- a/pkg/app/piped/executor/ecs/ecs.go +++ b/pkg/app/piped/executor/ecs/ecs.go @@ -457,7 +457,14 @@ func routing(ctx context.Context, in *executor.Input, platformProviderName strin return false } - if err := client.ModifyListenerOrRule(ctx, currListenerArns, currListenerRuleArns, routingTrafficCfg); err != nil { + if len(currListenerRuleArns) > 0 { + if err := client.ModifyRules(ctx, currListenerRuleArns, routingTrafficCfg); err != nil { + in.LogPersister.Errorf("Failed to routing traffic to PRIMARY/CANARY variants: %v", err) + return false + } + } + + if err := client.ModifyListeners(ctx, currListenerArns, routingTrafficCfg); err != nil { in.LogPersister.Errorf("Failed to routing traffic to PRIMARY/CANARY variants: %v", err) return false } diff --git a/pkg/app/piped/executor/ecs/rollback.go b/pkg/app/piped/executor/ecs/rollback.go index 7a28211e12..067514f40c 100644 --- a/pkg/app/piped/executor/ecs/rollback.go +++ b/pkg/app/piped/executor/ecs/rollback.go @@ -164,7 +164,14 @@ func rollback(ctx context.Context, in *executor.Input, platformProviderName stri return false } - if err := client.ModifyListenerOrRule(ctx, currListenerArns, currListenerRuleArns, routingTrafficCfg); err != nil { + if len(currListenerRuleArns) > 0 { + if err := client.ModifyRules(ctx, currListenerRuleArns, routingTrafficCfg); err != nil { + in.LogPersister.Errorf("Failed to routing traffic to PRIMARY/CANARY variants: %v", err) + return false + } + } + + if err := client.ModifyListeners(ctx, currListenerArns, routingTrafficCfg); err != nil { in.LogPersister.Errorf("Failed to routing traffic to PRIMARY/CANARY variants: %v", err) return false } diff --git a/pkg/app/piped/platformprovider/ecs/client.go b/pkg/app/piped/platformprovider/ecs/client.go index 5d678d15b9..1075a1e94e 100644 --- a/pkg/app/piped/platformprovider/ecs/client.go +++ b/pkg/app/piped/platformprovider/ecs/client.go @@ -456,125 +456,101 @@ func (c *client) getLoadBalancerArn(ctx context.Context, targetGroupArn string) return output.TargetGroups[0].LoadBalancerArns[0], nil } -func (c *client) ModifyListenerOrRule(ctx context.Context, listenerArns []string, listenerRuleArns []string, routingTrafficCfg RoutingTrafficConfig) error { +func (c *client) ModifyListeners(ctx context.Context, listenerArns []string, routingTrafficCfg RoutingTrafficConfig) error { if len(routingTrafficCfg) != 2 { return fmt.Errorf("invalid listener configuration: requires 2 target groups") } - if len(listenerRuleArns) > 0 { - modifyListenerRule := func(ctx context.Context, listenerRuleArn string) error { - input := &elasticloadbalancingv2.ModifyRuleInput{ - RuleArn: aws.String(listenerRuleArn), - Actions: []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)), - }, + modifyListener := func(ctx context.Context, listenerArn string) error { + input := &elasticloadbalancingv2.ModifyListenerInput{ + ListenerArn: aws.String(listenerArn), + DefaultActions: []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)), }, }, }, }, - } - _, err := c.elbClient.ModifyRule(ctx, input) - return err + }, } + _, err := c.elbClient.ModifyListener(ctx, input) + return err + } - for _, ruleArn := range listenerRuleArns { - // Describe the rule to get current actions - describeRulesOutput, err := c.elbClient.DescribeRules(ctx, &elasticloadbalancingv2.DescribeRulesInput{ - RuleArns: []string{ruleArn}, - }) - if err != nil { - return fmt.Errorf("error describing listener rule %v: %w", ruleArn, err) - } - - // No rules found, nothing to modify - if len(describeRulesOutput.Rules) == 0 { - return fmt.Errorf("no rules found for ARN: %s", ruleArn) - } - - // Check if the current action type is forward - for _, rule := range describeRulesOutput.Rules { - for _, action := range rule.Actions { - if action.Type == elbtypes.ActionTypeEnumForward { - // Call modifyListenerRule only if action type is forward - if err := modifyListenerRule(ctx, ruleArn); err != nil { - return err - } - // Break the loop once the modify function is called for the rule - break - } - } - } + for _, listener := range listenerArns { + if err := modifyListener(ctx, listener); err != nil { + return err } + } + return nil +} + +func (c *client) ModifyListenerOrRule(ctx context.Context, listenerRuleArns []string, routingTrafficCfg RoutingTrafficConfig) error { + if len(routingTrafficCfg) != 2 { + return fmt.Errorf("invalid listener configuration: requires 2 target groups") + } - return nil - } - - if len(listenerRuleArns) == 0 { - modifyListener := func(ctx context.Context, listenerArn string) error { - input := &elasticloadbalancingv2.ModifyListenerInput{ - ListenerArn: aws.String(listenerArn), - DefaultActions: []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)), - }, + modifyListenerRule := func(ctx context.Context, listenerRuleArn string) error { + input := &elasticloadbalancingv2.ModifyRuleInput{ + RuleArn: aws.String(listenerRuleArn), + Actions: []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)), }, }, }, }, - } - _, err := c.elbClient.ModifyListener(ctx, input) - return err + }, } + _, err := c.elbClient.ModifyRule(ctx, input) + return err + } - for _, listenerArn := range listenerArns { - // Describe the listener to get current default actions - describeListenerOutput, err := c.elbClient.DescribeListeners(ctx, &elasticloadbalancingv2.DescribeListenersInput{ - ListenerArns: []string{listenerArn}, - }) - if err != nil { - return fmt.Errorf("error describing listener %v: %w", listenerArn, err) - } + for _, ruleArn := range listenerRuleArns { + // Describe the rule to get current actions + describeRulesOutput, err := c.elbClient.DescribeRules(ctx, &elasticloadbalancingv2.DescribeRulesInput{ + RuleArns: []string{ruleArn}, + }) + if err != nil { + return fmt.Errorf("error describing listener rule %v: %w", ruleArn, err) + } - // No listeners found, nothing to modify - if len(describeListenerOutput.Listeners) == 0 { - return fmt.Errorf("no listeners found for ARN: %s", listenerArn) - } + // No rules found, nothing to modify + if len(describeRulesOutput.Rules) == 0 { + return fmt.Errorf("no rules found for ARN: %s", ruleArn) + } - // Check if the current default action type is forward - for _, listener := range describeListenerOutput.Listeners { - for _, action := range listener.DefaultActions { - if action.Type == elbtypes.ActionTypeEnumForward { - // Call modifyListener only if default action type is forward - if err := modifyListener(ctx, listenerArn); err != nil { - return err - } - // Break the loop once the modify function is called for the listener - break + // Check if the current action type is forward + for _, rule := range describeRulesOutput.Rules { + for _, action := range rule.Actions { + if action.Type == elbtypes.ActionTypeEnumForward { + // Call modifyListenerRule only if action type is forward + if err := modifyListenerRule(ctx, ruleArn); err != nil { + return err } + // Break the loop once the modify function is called for the rule + break } } } - - return nil } return nil diff --git a/pkg/app/piped/platformprovider/ecs/ecs.go b/pkg/app/piped/platformprovider/ecs/ecs.go index dda41a07e3..c819c58659 100644 --- a/pkg/app/piped/platformprovider/ecs/ecs.go +++ b/pkg/app/piped/platformprovider/ecs/ecs.go @@ -58,7 +58,8 @@ type ECS interface { type ELB interface { GetListenerArns(ctx context.Context, targetGroup types.LoadBalancer) ([]string, error) GetListenerRuleArns(ctx context.Context, listenerArns []string) ([]string, error) - ModifyListenerOrRule(ctx context.Context, listenerArns []string, listenerRuleArns []string, routingTrafficCfg RoutingTrafficConfig) error + ModifyListeners(ctx context.Context, listenerArns []string, routingTrafficCfg RoutingTrafficConfig) error + ModifyRules(ctx context.Context, listenerRuleArns []string, routingTrafficCfg RoutingTrafficConfig) error } // Registry holds a pool of aws client wrappers. From 3a4bea5ab8f0f8baf79ea9734b6a91563c9424e8 Mon Sep 17 00:00:00 2001 From: moko-poi Date: Fri, 24 Nov 2023 13:13:47 +0900 Subject: [PATCH 07/11] fix: ModifyRules Signed-off-by: moko-poi --- pkg/app/piped/platformprovider/ecs/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/app/piped/platformprovider/ecs/client.go b/pkg/app/piped/platformprovider/ecs/client.go index 1075a1e94e..72e7ffca66 100644 --- a/pkg/app/piped/platformprovider/ecs/client.go +++ b/pkg/app/piped/platformprovider/ecs/client.go @@ -494,7 +494,7 @@ func (c *client) ModifyListeners(ctx context.Context, listenerArns []string, rou return nil } -func (c *client) ModifyListenerOrRule(ctx context.Context, listenerRuleArns []string, routingTrafficCfg RoutingTrafficConfig) error { +func (c *client) ModifyRules(ctx context.Context, listenerRuleArns []string, routingTrafficCfg RoutingTrafficConfig) error { if len(routingTrafficCfg) != 2 { return fmt.Errorf("invalid listener configuration: requires 2 target groups") } From 0b01246eabf38c6c9035c755e7d59727001ae5e0 Mon Sep 17 00:00:00 2001 From: moko-poi Date: Fri, 24 Nov 2023 13:16:36 +0900 Subject: [PATCH 08/11] fix: rollback Signed-off-by: moko-poi --- pkg/app/piped/executor/ecs/rollback.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/app/piped/executor/ecs/rollback.go b/pkg/app/piped/executor/ecs/rollback.go index 067514f40c..16699f6ded 100644 --- a/pkg/app/piped/executor/ecs/rollback.go +++ b/pkg/app/piped/executor/ecs/rollback.go @@ -169,6 +169,8 @@ func rollback(ctx context.Context, in *executor.Input, platformProviderName stri in.LogPersister.Errorf("Failed to routing traffic to PRIMARY/CANARY variants: %v", err) return false } + + return true } if err := client.ModifyListeners(ctx, currListenerArns, routingTrafficCfg); err != nil { From abd12ca3edf73067525c6bf9be90757b91938485 Mon Sep 17 00:00:00 2001 From: moko-poi Date: Fri, 24 Nov 2023 17:40:31 +0900 Subject: [PATCH 09/11] fix: routing Signed-off-by: moko-poi --- pkg/app/piped/executor/ecs/ecs.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/app/piped/executor/ecs/ecs.go b/pkg/app/piped/executor/ecs/ecs.go index 9c5a80f112..ceef6b8c42 100644 --- a/pkg/app/piped/executor/ecs/ecs.go +++ b/pkg/app/piped/executor/ecs/ecs.go @@ -462,6 +462,8 @@ func routing(ctx context.Context, in *executor.Input, platformProviderName strin in.LogPersister.Errorf("Failed to routing traffic to PRIMARY/CANARY variants: %v", err) return false } + + return true } if err := client.ModifyListeners(ctx, currListenerArns, routingTrafficCfg); err != nil { From 38855ce1aed448a80309d1466e2fb77f50ac92a3 Mon Sep 17 00:00:00 2001 From: moko-poi Date: Fri, 24 Nov 2023 18:30:33 +0900 Subject: [PATCH 10/11] fix: action.Type == elbtypes.ActionTypeEnumForward Signed-off-by: moko-poi --- pkg/app/piped/platformprovider/ecs/client.go | 54 ++++++++++++++------ 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/pkg/app/piped/platformprovider/ecs/client.go b/pkg/app/piped/platformprovider/ecs/client.go index 72e7ffca66..9cb7064b16 100644 --- a/pkg/app/piped/platformprovider/ecs/client.go +++ b/pkg/app/piped/platformprovider/ecs/client.go @@ -18,6 +18,7 @@ import ( "context" "errors" "fmt" + "strings" "time" "github.com/aws/aws-sdk-go-v2/aws" @@ -461,6 +462,19 @@ func (c *client) ModifyListeners(ctx context.Context, listenerArns []string, rou return fmt.Errorf("invalid listener configuration: requires 2 target groups") } + // Describe the rule to get current actions + describelistenerOutput, err := c.elbClient.DescribeRules(ctx, &elasticloadbalancingv2.DescribeRulesInput{ + RuleArns: listenerArns, + }) + if err != nil { + return fmt.Errorf("error describing listener %v: %w", strings.Join(listenerArns, ", "), err) + } + + // No rules found, nothing to modify + if len(describelistenerOutput.Rules) == 0 { + return fmt.Errorf("no listener found for ARN: %s", strings.Join(listenerArns, ", ")) + } + modifyListener := func(ctx context.Context, listenerArn string) error { input := &elasticloadbalancingv2.ModifyListenerInput{ ListenerArn: aws.String(listenerArn), @@ -487,8 +501,16 @@ func (c *client) ModifyListeners(ctx context.Context, listenerArns []string, rou } for _, listener := range listenerArns { - if err := modifyListener(ctx, listener); err != nil { - return err + // Check if the current action type is forward + for _, rule := range describelistenerOutput.Rules { + for _, action := range rule.Actions { + if action.Type == elbtypes.ActionTypeEnumForward { + // Call modifyListenerRule only if action type is forward + if err := modifyListener(ctx, listener); err != nil { + return err + } + } + } } } return nil @@ -499,6 +521,19 @@ func (c *client) ModifyRules(ctx context.Context, listenerRuleArns []string, rou return fmt.Errorf("invalid listener configuration: requires 2 target groups") } + // Describe the rule to get current actions + describeRulesOutput, err := c.elbClient.DescribeRules(ctx, &elasticloadbalancingv2.DescribeRulesInput{ + RuleArns: listenerRuleArns, + }) + if err != nil { + return fmt.Errorf("error describing listener rule %v: %w", strings.Join(listenerRuleArns, ", "), err) + } + + // No rules found, nothing to modify + if len(describeRulesOutput.Rules) == 0 { + return fmt.Errorf("no rules found for ARN: %s", strings.Join(listenerRuleArns, ", ")) + } + modifyListenerRule := func(ctx context.Context, listenerRuleArn string) error { input := &elasticloadbalancingv2.ModifyRuleInput{ RuleArn: aws.String(listenerRuleArn), @@ -525,19 +560,6 @@ func (c *client) ModifyRules(ctx context.Context, listenerRuleArns []string, rou } for _, ruleArn := range listenerRuleArns { - // Describe the rule to get current actions - describeRulesOutput, err := c.elbClient.DescribeRules(ctx, &elasticloadbalancingv2.DescribeRulesInput{ - RuleArns: []string{ruleArn}, - }) - if err != nil { - return fmt.Errorf("error describing listener rule %v: %w", ruleArn, err) - } - - // No rules found, nothing to modify - if len(describeRulesOutput.Rules) == 0 { - return fmt.Errorf("no rules found for ARN: %s", ruleArn) - } - // Check if the current action type is forward for _, rule := range describeRulesOutput.Rules { for _, action := range rule.Actions { @@ -546,8 +568,6 @@ func (c *client) ModifyRules(ctx context.Context, listenerRuleArns []string, rou if err := modifyListenerRule(ctx, ruleArn); err != nil { return err } - // Break the loop once the modify function is called for the rule - break } } } From 2650922af2c52c64cac7429973eb22966dd4b6d8 Mon Sep 17 00:00:00 2001 From: moko-poi Date: Sun, 26 Nov 2023 11:06:54 +0900 Subject: [PATCH 11/11] fix: Update ModifyListeners and ModifyRules for Selective Action Modification Signed-off-by: moko-poi --- pkg/app/piped/platformprovider/ecs/client.go | 121 +++++++++---------- 1 file changed, 54 insertions(+), 67 deletions(-) diff --git a/pkg/app/piped/platformprovider/ecs/client.go b/pkg/app/piped/platformprovider/ecs/client.go index 9cb7064b16..c92f4d17e1 100644 --- a/pkg/app/piped/platformprovider/ecs/client.go +++ b/pkg/app/piped/platformprovider/ecs/client.go @@ -462,24 +462,21 @@ func (c *client) ModifyListeners(ctx context.Context, listenerArns []string, rou return fmt.Errorf("invalid listener configuration: requires 2 target groups") } - // Describe the rule to get current actions - describelistenerOutput, err := c.elbClient.DescribeRules(ctx, &elasticloadbalancingv2.DescribeRulesInput{ - RuleArns: listenerArns, - }) - if err != nil { - return fmt.Errorf("error describing listener %v: %w", strings.Join(listenerArns, ", "), err) - } - - // No rules found, nothing to modify - if len(describelistenerOutput.Rules) == 0 { - return fmt.Errorf("no listener found for ARN: %s", strings.Join(listenerArns, ", ")) - } + for _, listenerArn := range listenerArns { + // Describe the listener to get the current actions + describeListenersOutput, err := c.elbClient.DescribeListeners(ctx, &elasticloadbalancingv2.DescribeListenersInput{ + ListenerArns: []string{listenerArn}, + }) + if err != nil { + return fmt.Errorf("error describing listener %s: %w", listenerArn, err) + } - modifyListener := func(ctx context.Context, listenerArn string) error { - input := &elasticloadbalancingv2.ModifyListenerInput{ - ListenerArn: aws.String(listenerArn), - DefaultActions: []elbtypes.Action{ - { + // 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 (new logic) + modifiedAction := elbtypes.Action{ Type: elbtypes.ActionTypeEnumForward, ForwardConfig: &elbtypes.ForwardActionConfig{ TargetGroups: []elbtypes.TargetGroupTuple{ @@ -493,25 +490,22 @@ func (c *client) ModifyListeners(ctx context.Context, listenerArns []string, rou }, }, }, - }, - }, - } - _, err := c.elbClient.ModifyListener(ctx, input) - return err - } - - for _, listener := range listenerArns { - // Check if the current action type is forward - for _, rule := range describelistenerOutput.Rules { - for _, action := range rule.Actions { - if action.Type == elbtypes.ActionTypeEnumForward { - // Call modifyListenerRule only if action type is forward - if err := modifyListener(ctx, listener); err != nil { - return err - } } + modifiedActions = append(modifiedActions, modifiedAction) + } else { + // Keep other actions unchanged (new logic) + 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) + } } return nil } @@ -521,24 +515,21 @@ func (c *client) ModifyRules(ctx context.Context, listenerRuleArns []string, rou return fmt.Errorf("invalid listener configuration: requires 2 target groups") } - // Describe the rule to get current actions - describeRulesOutput, err := c.elbClient.DescribeRules(ctx, &elasticloadbalancingv2.DescribeRulesInput{ - RuleArns: listenerRuleArns, - }) - if err != nil { - return fmt.Errorf("error describing listener rule %v: %w", strings.Join(listenerRuleArns, ", "), err) - } - - // No rules found, nothing to modify - if len(describeRulesOutput.Rules) == 0 { - return fmt.Errorf("no rules found for ARN: %s", strings.Join(listenerRuleArns, ", ")) - } + for _, ruleArn := range listenerRuleArns { + // Describe the rule to get current actions + describeRulesOutput, err := c.elbClient.DescribeRules(ctx, &elasticloadbalancingv2.DescribeRulesInput{ + RuleArns: []string{ruleArn}, + }) + if err != nil { + return fmt.Errorf("error describing listener rule %v: %w", strings.Join(listenerRuleArns, ", "), err) + } - modifyListenerRule := func(ctx context.Context, listenerRuleArn string) error { - input := &elasticloadbalancingv2.ModifyRuleInput{ - RuleArn: aws.String(listenerRuleArn), - Actions: []elbtypes.Action{ - { + // Prepare the actions to be modified + var modifiedActions []elbtypes.Action + for _, action := range describeRulesOutput.Rules[0].Actions { + if action.Type == elbtypes.ActionTypeEnumForward { + // Modify only the forward action (new logic) + modifiedAction := elbtypes.Action{ Type: elbtypes.ActionTypeEnumForward, ForwardConfig: &elbtypes.ForwardActionConfig{ TargetGroups: []elbtypes.TargetGroupTuple{ @@ -552,27 +543,23 @@ func (c *client) ModifyRules(ctx context.Context, listenerRuleArns []string, rou }, }, }, - }, - }, - } - _, err := c.elbClient.ModifyRule(ctx, input) - return err - } - - for _, ruleArn := range listenerRuleArns { - // Check if the current action type is forward - for _, rule := range describeRulesOutput.Rules { - for _, action := range rule.Actions { - if action.Type == elbtypes.ActionTypeEnumForward { - // Call modifyListenerRule only if action type is forward - if err := modifyListenerRule(ctx, ruleArn); err != nil { - return err - } } + modifiedActions = append(modifiedActions, modifiedAction) + } else { + // Keep other actions unchanged (new logic) + modifiedActions = append(modifiedActions, action) } } - } + // Modify the rule with the new actions + _, err = c.elbClient.ModifyRule(ctx, &elasticloadbalancingv2.ModifyRuleInput{ + RuleArn: aws.String(ruleArn), + Actions: modifiedActions, + }) + if err != nil { + return fmt.Errorf("error modifying listener rule %s: %w", ruleArn, err) + } + } return nil }