From 0d0286794a21869ce2bfcd8f8811dc3dc556280a Mon Sep 17 00:00:00 2001 From: jessemeng Date: Sat, 6 Jun 2026 21:58:16 +0800 Subject: [PATCH 1/4] feat: add target-management annotation to skip target reconciliation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the 'elbv2.services.k8s.aws/target-management: ignore' annotation that tells the controller to leave targets alone — no DescribeTargetHealth, no RegisterTargets, no DeregisterTargets. This allows an external controller to own target registration while ACK still manages the target group itself (health checks, attributes, etc.). Three guard points: - sdkFind: skip describeTargets so externally-registered targets are never read from AWS (no drift detected) - sdkUpdate: guard the register/deregister block with the annotation check - sdkCreate: skip the post-create requeue for target registration Templates updated so future code regeneration preserves the behavior. Co-Authored-By: Claude Opus 4.8 --- pkg/resource/target_group/hooks.go | 22 +++ pkg/resource/target_group/hooks_test.go | 151 ++++++++++++++++++ pkg/resource/target_group/sdk.go | 14 +- .../sdk_create_post_set_output.go.tpl | 2 +- .../sdk_read_many_post_set_output.go.tpl | 10 +- .../sdk_update_pre_build_request.go.tpl | 2 +- 6 files changed, 191 insertions(+), 10 deletions(-) diff --git a/pkg/resource/target_group/hooks.go b/pkg/resource/target_group/hooks.go index 96af811..d9a3e8c 100644 --- a/pkg/resource/target_group/hooks.go +++ b/pkg/resource/target_group/hooks.go @@ -26,10 +26,32 @@ import ( svcsdktypes "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types" ) +const ( + // AnnotationTargetManagement controls how the controller manages targets + // registered with the target group. When set to "ignore", the controller + // will not read, register, or deregister targets — allowing an external + // controller to manage target registration independently. + AnnotationTargetManagement = "elbv2.services.k8s.aws/target-management" +) + var ( RequeueAfterUpdateDuration = 5 * time.Second ) +// isTargetManagementIgnored returns true if the resource has the +// AnnotationTargetManagement annotation set to "ignore", indicating that +// target registration should be managed by an external controller. +func isTargetManagementIgnored(r *resource) bool { + if r == nil || r.ko == nil { + return false + } + annotations := r.ko.GetAnnotations() + if annotations == nil { + return false + } + return annotations[AnnotationTargetManagement] == "ignore" +} + func customCompare( delta *ackcompare.Delta, a *resource, diff --git a/pkg/resource/target_group/hooks_test.go b/pkg/resource/target_group/hooks_test.go index 9e0f22d..0f412db 100644 --- a/pkg/resource/target_group/hooks_test.go +++ b/pkg/resource/target_group/hooks_test.go @@ -24,6 +24,10 @@ func ptr(s string) *string { return &s } +func int64Ptr(i int64) *int64 { + return &i +} + func TestContainsExactTargetGroupAttribute(t *testing.T) { attributes := []*svcapitypes.TargetGroupAttribute{ {Key: ptr("proxy_protocol_v2.enabled"), Value: ptr("true")}, @@ -352,3 +356,150 @@ func TestMultiAttributeDriftScenario(t *testing.T) { } }) } + +func TestIsTargetManagementIgnored(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + expected bool + }{ + { + name: "nil annotations", + annotations: nil, + expected: false, + }, + { + name: "empty annotations", + annotations: map[string]string{}, + expected: false, + }, + { + name: "annotation set to ignore", + annotations: map[string]string{ + "elbv2.services.k8s.aws/target-management": "ignore", + }, + expected: true, + }, + { + name: "annotation set to other value", + annotations: map[string]string{ + "elbv2.services.k8s.aws/target-management": "managed", + }, + expected: false, + }, + { + name: "other annotations present but not target-management", + annotations: map[string]string{ + "some.other.annotation": "value", + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tg := &svcapitypes.TargetGroup{} + tg.SetAnnotations(tt.annotations) + r := &resource{ko: tg} + result := isTargetManagementIgnored(r) + if result != tt.expected { + t.Errorf("isTargetManagementIgnored() = %v, want %v", result, tt.expected) + } + }) + } +} + +func TestIsTargetManagementIgnoredNilResource(t *testing.T) { + if isTargetManagementIgnored(nil) { + t.Error("expected false for nil resource") + } + + r := &resource{ko: nil} + if isTargetManagementIgnored(r) { + t.Error("expected false for resource with nil ko") + } +} + +// TestCompareTargetDescriptionWithIgnoredAnnotation verifies that when target +// management is ignored and sdkFind skips describeTargets, both desired and +// latest have the same targets (from the k8s spec), resulting in no delta. +func TestCompareTargetDescriptionWithIgnoredAnnotation(t *testing.T) { + t.Run("both nil targets - no delta", func(t *testing.T) { + delta := ackcompare.NewDelta() + desired := &resource{ko: &svcapitypes.TargetGroup{ + Spec: svcapitypes.TargetGroupSpec{ + Targets: nil, + }, + }} + latest := &resource{ko: &svcapitypes.TargetGroup{ + Spec: svcapitypes.TargetGroupSpec{ + Targets: nil, + }, + }} + compareTargetDescription(delta, desired, latest) + if len(delta.Differences) > 0 { + t.Error("expected no delta when both desired and latest have nil targets") + } + }) + + t.Run("same non-empty targets - no delta", func(t *testing.T) { + delta := ackcompare.NewDelta() + targets := []*svcapitypes.TargetDescription{ + {ID: ptr("i-12345"), Port: int64Ptr(80)}, + } + desired := &resource{ko: &svcapitypes.TargetGroup{ + Spec: svcapitypes.TargetGroupSpec{ + Targets: targets, + }, + }} + latest := &resource{ko: &svcapitypes.TargetGroup{ + Spec: svcapitypes.TargetGroupSpec{ + Targets: targets, + }, + }} + compareTargetDescription(delta, desired, latest) + if len(delta.Differences) > 0 { + t.Error("expected no delta when desired and latest have identical targets") + } + }) + + t.Run("desired empty, latest has targets - delta (simulates annotation NOT set)", func(t *testing.T) { + delta := ackcompare.NewDelta() + desired := &resource{ko: &svcapitypes.TargetGroup{ + Spec: svcapitypes.TargetGroupSpec{ + Targets: nil, + }, + }} + latest := &resource{ko: &svcapitypes.TargetGroup{ + Spec: svcapitypes.TargetGroupSpec{ + Targets: []*svcapitypes.TargetDescription{ + {ID: ptr("i-external")}, + }, + }, + }} + compareTargetDescription(delta, desired, latest) + if len(delta.Differences) == 0 { + t.Error("expected delta when desired is empty and latest has targets (annotation not set)") + } + }) + + t.Run("desired has targets, latest empty - delta", func(t *testing.T) { + delta := ackcompare.NewDelta() + desired := &resource{ko: &svcapitypes.TargetGroup{ + Spec: svcapitypes.TargetGroupSpec{ + Targets: []*svcapitypes.TargetDescription{ + {ID: ptr("i-new-target")}, + }, + }, + }} + latest := &resource{ko: &svcapitypes.TargetGroup{ + Spec: svcapitypes.TargetGroupSpec{ + Targets: nil, + }, + }} + compareTargetDescription(delta, desired, latest) + if len(delta.Differences) == 0 { + t.Error("expected delta when desired has targets and latest is empty") + } + }) +} diff --git a/pkg/resource/target_group/sdk.go b/pkg/resource/target_group/sdk.go index d49a2e3..9b6187b 100644 --- a/pkg/resource/target_group/sdk.go +++ b/pkg/resource/target_group/sdk.go @@ -204,9 +204,13 @@ func (rm *resourceManager) sdkFind( } rm.setStatusDefaults(ko) - err = rm.describeTargets(ctx, &resource{ko}) - if err != nil { - return nil, err + // When target management is ignored, skip reading targets from AWS so that + // externally registered targets are not treated as drift. + if !isTargetManagementIgnored(r) { + err = rm.describeTargets(ctx, &resource{ko}) + if err != nil { + return nil, err + } } rm.setStatusDefaults(ko) @@ -388,7 +392,7 @@ func (rm *resourceManager) sdkCreate( } rm.setStatusDefaults(ko) - if ko.Spec.Targets != nil || len(ko.Spec.Attributes) > 0 { + if (ko.Spec.Targets != nil && !isTargetManagementIgnored(desired)) || len(ko.Spec.Attributes) > 0 { return nil, ackrequeue.NeededAfter(fmt.Errorf("Requeuing for post-create updates (targets or attributes)"), RequeueAfterUpdateDuration) } @@ -514,7 +518,7 @@ func (rm *resourceManager) sdkUpdate( defer func() { exit(err) }() - if delta.DifferentAt("Spec.Targets") { + if delta.DifferentAt("Spec.Targets") && !isTargetManagementIgnored(desired) { added, removed := getTargetsDifference(latest.ko.Spec.Targets, desired.ko.Spec.Targets) if latest.ko.Status.ACKResourceMetadata == nil || latest.ko.Status.ACKResourceMetadata.ARN == nil { return nil, fmt.Errorf("target group ARN is not yet available") diff --git a/templates/hooks/target_group/sdk_create_post_set_output.go.tpl b/templates/hooks/target_group/sdk_create_post_set_output.go.tpl index e6117af..d1e810f 100644 --- a/templates/hooks/target_group/sdk_create_post_set_output.go.tpl +++ b/templates/hooks/target_group/sdk_create_post_set_output.go.tpl @@ -1,3 +1,3 @@ - if ko.Spec.Targets != nil || len(ko.Spec.Attributes) > 0 { + if (ko.Spec.Targets != nil && !isTargetManagementIgnored(desired)) || len(ko.Spec.Attributes) > 0 { return nil, ackrequeue.NeededAfter(fmt.Errorf("Requeuing for post-create updates (targets or attributes)"), RequeueAfterUpdateDuration) } diff --git a/templates/hooks/target_group/sdk_read_many_post_set_output.go.tpl b/templates/hooks/target_group/sdk_read_many_post_set_output.go.tpl index 8f46679..f87b9e7 100644 --- a/templates/hooks/target_group/sdk_read_many_post_set_output.go.tpl +++ b/templates/hooks/target_group/sdk_read_many_post_set_output.go.tpl @@ -1,6 +1,10 @@ - err = rm.describeTargets(ctx, &resource{ko}) - if err != nil { - return nil, err + // When target management is ignored, skip reading targets from AWS so that + // externally registered targets are not treated as drift. + if !isTargetManagementIgnored(r) { + err = rm.describeTargets(ctx, &resource{ko}) + if err != nil { + return nil, err + } } rm.setStatusDefaults(ko) diff --git a/templates/hooks/target_group/sdk_update_pre_build_request.go.tpl b/templates/hooks/target_group/sdk_update_pre_build_request.go.tpl index 5742d9c..5f32873 100644 --- a/templates/hooks/target_group/sdk_update_pre_build_request.go.tpl +++ b/templates/hooks/target_group/sdk_update_pre_build_request.go.tpl @@ -1,4 +1,4 @@ - if delta.DifferentAt("Spec.Targets") { + if delta.DifferentAt("Spec.Targets") && !isTargetManagementIgnored(desired) { added, removed := getTargetsDifference(latest.ko.Spec.Targets, desired.ko.Spec.Targets) if latest.ko.Status.ACKResourceMetadata == nil || latest.ko.Status.ACKResourceMetadata.ARN == nil { return nil, fmt.Errorf("target group ARN is not yet available") From ebfc70cedd9a1127242911cc6a6fef97cabf09a3 Mon Sep 17 00:00:00 2001 From: jessemeng Date: Sat, 6 Jun 2026 22:06:59 +0800 Subject: [PATCH 2/4] feat: add weight-management annotation to skip weight reconciliation for listeners Add the 'elbv2.services.k8s.aws/weight-management: ignore' annotation that tells the controller to not reconcile forward action target group weights. This allows external blue/green deployment tools to manage traffic shifting independently while ACK still manages the rest of the listener configuration. Uses a delta_pre_compare hook (customPreCompare) that copies AWS-side weights into the desired spec before DeepEqual comparison, so weight differences never generate deltas. Since the desired spec is mutated with the latest weights, any ModifyListener calls triggered by other field changes also preserve the AWS-side weights. Co-Authored-By: Claude Opus 4.8 --- apis/v1alpha1/generator.yaml | 2 + generator.yaml | 2 + pkg/resource/listener/delta.go | 1 + pkg/resource/listener/hooks.go | 74 +++++++ pkg/resource/listener/hooks_test.go | 326 ++++++++++++++++++++++++++++ 5 files changed, 405 insertions(+) create mode 100644 pkg/resource/listener/hooks_test.go diff --git a/apis/v1alpha1/generator.yaml b/apis/v1alpha1/generator.yaml index 0a32afd..4514a1e 100644 --- a/apis/v1alpha1/generator.yaml +++ b/apis/v1alpha1/generator.yaml @@ -165,6 +165,8 @@ resources: tags: ignore: true hooks: + delta_pre_compare: + code: customPreCompare(delta, a, b) sdk_read_many_post_build_request: template_path: hooks/listener/sdk_read_many_post_build_request.go.tpl TargetGroup: diff --git a/generator.yaml b/generator.yaml index 0a32afd..4514a1e 100644 --- a/generator.yaml +++ b/generator.yaml @@ -165,6 +165,8 @@ resources: tags: ignore: true hooks: + delta_pre_compare: + code: customPreCompare(delta, a, b) sdk_read_many_post_build_request: template_path: hooks/listener/sdk_read_many_post_build_request.go.tpl TargetGroup: diff --git a/pkg/resource/listener/delta.go b/pkg/resource/listener/delta.go index c306edb..68ea7e9 100644 --- a/pkg/resource/listener/delta.go +++ b/pkg/resource/listener/delta.go @@ -41,6 +41,7 @@ func newResourceDelta( delta.Add("", a, b) return delta } + customPreCompare(delta, a, b) if len(a.ko.Spec.AlpnPolicy) != len(b.ko.Spec.AlpnPolicy) { delta.Add("Spec.AlpnPolicy", a.ko.Spec.AlpnPolicy, b.ko.Spec.AlpnPolicy) diff --git a/pkg/resource/listener/hooks.go b/pkg/resource/listener/hooks.go index 2be33ef..2105234 100644 --- a/pkg/resource/listener/hooks.go +++ b/pkg/resource/listener/hooks.go @@ -1,5 +1,17 @@ package listener +import ( + ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare" +) + +const ( + // AnnotationWeightManagement controls how the controller manages forward + // action weights for target groups. When set to "ignore", the controller + // will not reconcile weight differences — allowing an external controller + // or deployment tool to manage blue/green traffic shifting independently. + AnnotationWeightManagement = "elbv2.services.k8s.aws/weight-management" +) + // customCheckRequiredFieldsMissingMethod returns true if there are any fields // for the ReadOne Input shape that are required but not present in the // resource's Spec or Status. @@ -8,3 +20,65 @@ func (rm *resourceManager) customCheckRequiredFieldsMissingMethod( ) bool { return r.Identifiers().ARN() == nil } + +// customPreCompare is the delta_pre_compare hook. When weight management is +// ignored, it copies the AWS-side weights into the desired spec so that the +// DeepEqual comparison on DefaultActions does not flag external weight +// changes as drift. +func customPreCompare( + delta *ackcompare.Delta, + a *resource, + b *resource, +) { + if isWeightManagementIgnored(a) { + mergeLatestWeights(a, b) + } +} + +// isWeightManagementIgnored returns true if the resource has the +// AnnotationWeightManagement annotation set to "ignore", indicating that +// target group weights should be managed by an external controller. +func isWeightManagementIgnored(r *resource) bool { + if r == nil || r.ko == nil { + return false + } + annotations := r.ko.GetAnnotations() + if annotations == nil { + return false + } + return annotations[AnnotationWeightManagement] == "ignore" +} + +// mergeLatestWeights copies the TargetGroup weights from the latest (AWS) +// state into the desired resource. This prevents external weight changes +// from being detected as drift and from being overwritten during updates. +func mergeLatestWeights(desired, latest *resource) { + if latest == nil || latest.ko == nil || desired == nil || desired.ko == nil { + return + } + // Build a map from TargetGroupARN to Weight from the latest (AWS) state + latestWeights := map[string]*int64{} + for _, action := range latest.ko.Spec.DefaultActions { + if action.ForwardConfig != nil { + for _, tg := range action.ForwardConfig.TargetGroups { + if tg.TargetGroupARN != nil { + latestWeights[*tg.TargetGroupARN] = tg.Weight + } + } + } + } + + // Overwrite desired weights with latest weights for any target group + // that exists in both desired and latest. + for _, action := range desired.ko.Spec.DefaultActions { + if action.ForwardConfig != nil { + for _, tg := range action.ForwardConfig.TargetGroups { + if tg.TargetGroupARN != nil { + if w, ok := latestWeights[*tg.TargetGroupARN]; ok { + tg.Weight = w + } + } + } + } + } +} diff --git a/pkg/resource/listener/hooks_test.go b/pkg/resource/listener/hooks_test.go new file mode 100644 index 0000000..e702d77 --- /dev/null +++ b/pkg/resource/listener/hooks_test.go @@ -0,0 +1,326 @@ +package listener + +import ( + "testing" + + svcapitypes "github.com/aws-controllers-k8s/elbv2-controller/apis/v1alpha1" +) + +func ptr(s string) *string { + return &s +} + +func int64Ptr(i int64) *int64 { + return &i +} + +func TestIsWeightManagementIgnored(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + expected bool + }{ + { + name: "nil annotations", + annotations: nil, + expected: false, + }, + { + name: "empty annotations", + annotations: map[string]string{}, + expected: false, + }, + { + name: "annotation set to ignore", + annotations: map[string]string{ + "elbv2.services.k8s.aws/weight-management": "ignore", + }, + expected: true, + }, + { + name: "annotation set to other value", + annotations: map[string]string{ + "elbv2.services.k8s.aws/weight-management": "managed", + }, + expected: false, + }, + { + name: "other annotations present but not weight-management", + annotations: map[string]string{ + "some.other.annotation": "value", + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + l := &svcapitypes.Listener{} + l.SetAnnotations(tt.annotations) + r := &resource{ko: l} + result := isWeightManagementIgnored(r) + if result != tt.expected { + t.Errorf("isWeightManagementIgnored() = %v, want %v", result, tt.expected) + } + }) + } +} + +func TestIsWeightManagementIgnoredNilResource(t *testing.T) { + if isWeightManagementIgnored(nil) { + t.Error("expected false for nil resource") + } + + r := &resource{ko: nil} + if isWeightManagementIgnored(r) { + t.Error("expected false for resource with nil ko") + } +} + +func TestMergeLatestWeights(t *testing.T) { + t.Run("merges weights for matching TGs", func(t *testing.T) { + desired := &resource{ko: &svcapitypes.Listener{ + Spec: svcapitypes.ListenerSpec{ + DefaultActions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(100)}, + {TargetGroupARN: ptr("arn:aws:tg:green"), Weight: int64Ptr(0)}, + }, + }, + }, + }, + }, + }} + latest := &resource{ko: &svcapitypes.Listener{ + Spec: svcapitypes.ListenerSpec{ + DefaultActions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(70)}, + {TargetGroupARN: ptr("arn:aws:tg:green"), Weight: int64Ptr(30)}, + }, + }, + }, + }, + }, + }} + + mergeLatestWeights(desired, latest) + + blueWeight := *desired.ko.Spec.DefaultActions[0].ForwardConfig.TargetGroups[0].Weight + greenWeight := *desired.ko.Spec.DefaultActions[0].ForwardConfig.TargetGroups[1].Weight + + if blueWeight != 70 { + t.Errorf("expected blue weight 70, got %d", blueWeight) + } + if greenWeight != 30 { + t.Errorf("expected green weight 30, got %d", greenWeight) + } + }) + + t.Run("does not change weights for TGs not in latest", func(t *testing.T) { + desired := &resource{ko: &svcapitypes.Listener{ + Spec: svcapitypes.ListenerSpec{ + DefaultActions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(100)}, + {TargetGroupARN: ptr("arn:aws:tg:new-green"), Weight: int64Ptr(0)}, + }, + }, + }, + }, + }, + }} + latest := &resource{ko: &svcapitypes.Listener{ + Spec: svcapitypes.ListenerSpec{ + DefaultActions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(70)}, + }, + }, + }, + }, + }, + }} + + mergeLatestWeights(desired, latest) + + blueWeight := *desired.ko.Spec.DefaultActions[0].ForwardConfig.TargetGroups[0].Weight + newGreenWeight := *desired.ko.Spec.DefaultActions[0].ForwardConfig.TargetGroups[1].Weight + + if blueWeight != 70 { + t.Errorf("expected blue weight 70 (merged from latest), got %d", blueWeight) + } + if newGreenWeight != 0 { + t.Errorf("expected new-green weight 0 (unchanged, not in latest), got %d", newGreenWeight) + } + }) + + t.Run("handles nil latest gracefully", func(t *testing.T) { + desired := &resource{ko: &svcapitypes.Listener{ + Spec: svcapitypes.ListenerSpec{ + DefaultActions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(100)}, + }, + }, + }, + }, + }, + }} + + // Should not panic + mergeLatestWeights(desired, nil) + mergeLatestWeights(nil, nil) + + weight := *desired.ko.Spec.DefaultActions[0].ForwardConfig.TargetGroups[0].Weight + if weight != 100 { + t.Errorf("expected weight unchanged (100), got %d", weight) + } + }) +} + +func TestCustomPreCompareWeightManagement(t *testing.T) { + t.Run("with annotation: copies weights, no delta on weight-only change", func(t *testing.T) { + desired := &resource{ko: &svcapitypes.Listener{ + Spec: svcapitypes.ListenerSpec{ + DefaultActions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(100)}, + {TargetGroupARN: ptr("arn:aws:tg:green"), Weight: int64Ptr(0)}, + }, + }, + }, + }, + }, + }} + desired.ko.SetAnnotations(map[string]string{ + "elbv2.services.k8s.aws/weight-management": "ignore", + }) + + latest := &resource{ko: &svcapitypes.Listener{ + Spec: svcapitypes.ListenerSpec{ + DefaultActions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(70)}, + {TargetGroupARN: ptr("arn:aws:tg:green"), Weight: int64Ptr(30)}, + }, + }, + }, + }, + }, + }} + + delta := newResourceDelta(desired, latest) + + // Weights should have been normalized — no delta on DefaultActions + if delta.DifferentAt("Spec.DefaultActions") { + t.Error("expected no delta on DefaultActions when weight-management is ignored") + } + }) + + t.Run("without annotation: detects weight differences as delta", func(t *testing.T) { + desired := &resource{ko: &svcapitypes.Listener{ + Spec: svcapitypes.ListenerSpec{ + DefaultActions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(100)}, + {TargetGroupARN: ptr("arn:aws:tg:green"), Weight: int64Ptr(0)}, + }, + }, + }, + }, + }, + }} + + latest := &resource{ko: &svcapitypes.Listener{ + Spec: svcapitypes.ListenerSpec{ + DefaultActions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(70)}, + {TargetGroupARN: ptr("arn:aws:tg:green"), Weight: int64Ptr(30)}, + }, + }, + }, + }, + }, + }} + + delta := newResourceDelta(desired, latest) + + // Without annotation, weight differences should be detected + if !delta.DifferentAt("Spec.DefaultActions") { + t.Error("expected delta on DefaultActions when weight-management is NOT set") + } + }) + + t.Run("with annotation: adding a new TG still creates delta", func(t *testing.T) { + desired := &resource{ko: &svcapitypes.Listener{ + Spec: svcapitypes.ListenerSpec{ + DefaultActions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(100)}, + {TargetGroupARN: ptr("arn:aws:tg:green"), Weight: int64Ptr(0)}, + }, + }, + }, + }, + }, + }} + desired.ko.SetAnnotations(map[string]string{ + "elbv2.services.k8s.aws/weight-management": "ignore", + }) + + latest := &resource{ko: &svcapitypes.Listener{ + Spec: svcapitypes.ListenerSpec{ + DefaultActions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(70)}, + // green is missing from latest + }, + }, + }, + }, + }, + }} + + delta := newResourceDelta(desired, latest) + + // Different number of TGs should still create a delta + if !delta.DifferentAt("Spec.DefaultActions") { + t.Error("expected delta when desired has an extra TG (even with weight-management ignored)") + } + }) +} From cbc43f7b3e303ff7a850ec7bc7089379e64546f2 Mon Sep 17 00:00:00 2001 From: jessemeng Date: Sun, 7 Jun 2026 20:31:37 +0800 Subject: [PATCH 3/4] fix: add rule weight-management & nil guard for action pointer - Add nil guard for action pointer in mergeLatestWeights (both listener and rule) since Actions/DefaultActions is []*Action, individual elements can be nil - Add weight-management annotation support to rule resources (isWeightManagementIgnored + mergeLatestWeights + customPreCompare hook) - Add tests for rule weight-management: annotation check, weight merge, and delta suppression with annotation - Fix: use string literal for annotation key in rule package to avoid duplicating listener's exported AnnotationWeightManagement constant Co-Authored-By: Claude Opus 4.8 --- pkg/resource/listener/hooks.go | 4 +- pkg/resource/rule/hooks.go | 51 ++++++ pkg/resource/rule/hooks_test.go | 310 ++++++++++++++++++++++++++++++++ 3 files changed, 363 insertions(+), 2 deletions(-) diff --git a/pkg/resource/listener/hooks.go b/pkg/resource/listener/hooks.go index 2105234..248017a 100644 --- a/pkg/resource/listener/hooks.go +++ b/pkg/resource/listener/hooks.go @@ -59,7 +59,7 @@ func mergeLatestWeights(desired, latest *resource) { // Build a map from TargetGroupARN to Weight from the latest (AWS) state latestWeights := map[string]*int64{} for _, action := range latest.ko.Spec.DefaultActions { - if action.ForwardConfig != nil { + if action != nil && action.ForwardConfig != nil { for _, tg := range action.ForwardConfig.TargetGroups { if tg.TargetGroupARN != nil { latestWeights[*tg.TargetGroupARN] = tg.Weight @@ -71,7 +71,7 @@ func mergeLatestWeights(desired, latest *resource) { // Overwrite desired weights with latest weights for any target group // that exists in both desired and latest. for _, action := range desired.ko.Spec.DefaultActions { - if action.ForwardConfig != nil { + if action != nil && action.ForwardConfig != nil { for _, tg := range action.ForwardConfig.TargetGroups { if tg.TargetGroupARN != nil { if w, ok := latestWeights[*tg.TargetGroupARN]; ok { diff --git a/pkg/resource/rule/hooks.go b/pkg/resource/rule/hooks.go index c561a1d..942bacc 100644 --- a/pkg/resource/rule/hooks.go +++ b/pkg/resource/rule/hooks.go @@ -90,6 +90,9 @@ func customPreCompare( a *resource, b *resource, ) { + if isWeightManagementIgnored(a) { + mergeLatestWeights(a, b) + } customCompareConditions(delta, a, b) } @@ -172,3 +175,51 @@ func customCompareConditions( } } } + +// isWeightManagementIgnored returns true if the resource has the +// AnnotationWeightManagement annotation set to "ignore", indicating that +// target group weights should be managed by an external controller. +func isWeightManagementIgnored(r *resource) bool { + if r == nil || r.ko == nil { + return false + } + annotations := r.ko.GetAnnotations() + if annotations == nil { + return false + } + return annotations["elbv2.services.k8s.aws/weight-management"] == "ignore" +} + +// mergeLatestWeights copies the TargetGroup weights from the latest (AWS) +// state into the desired resource. This prevents external weight changes +// from being detected as drift and from being overwritten during updates. +func mergeLatestWeights(desired, latest *resource) { + if latest == nil || latest.ko == nil || desired == nil || desired.ko == nil { + return + } + // Build a map from TargetGroupARN to Weight from the latest (AWS) state + latestWeights := map[string]*int64{} + for _, action := range latest.ko.Spec.Actions { + if action != nil && action.ForwardConfig != nil { + for _, tg := range action.ForwardConfig.TargetGroups { + if tg.TargetGroupARN != nil { + latestWeights[*tg.TargetGroupARN] = tg.Weight + } + } + } + } + + // Overwrite desired weights with latest weights for any target group + // that exists in both desired and latest. + for _, action := range desired.ko.Spec.Actions { + if action != nil && action.ForwardConfig != nil { + for _, tg := range action.ForwardConfig.TargetGroups { + if tg.TargetGroupARN != nil { + if w, ok := latestWeights[*tg.TargetGroupARN]; ok { + tg.Weight = w + } + } + } + } + } +} diff --git a/pkg/resource/rule/hooks_test.go b/pkg/resource/rule/hooks_test.go index 62fb099..2f3a5d7 100644 --- a/pkg/resource/rule/hooks_test.go +++ b/pkg/resource/rule/hooks_test.go @@ -23,6 +23,14 @@ import ( svcapitypes "github.com/aws-controllers-k8s/elbv2-controller/apis/v1alpha1" ) +func ptr(s string) *string { + return &s +} + +func int64Ptr(i int64) *int64 { + return &i +} + func TestCustomCompareConditions(t *testing.T) { tests := []struct { name string @@ -693,3 +701,305 @@ func TestCustomCompareConditions(t *testing.T) { }) } } + +func TestIsWeightManagementIgnored(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + expected bool + }{ + { + name: "nil annotations", + annotations: nil, + expected: false, + }, + { + name: "empty annotations", + annotations: map[string]string{}, + expected: false, + }, + { + name: "annotation set to ignore", + annotations: map[string]string{ + "elbv2.services.k8s.aws/weight-management": "ignore", + }, + expected: true, + }, + { + name: "annotation set to other value", + annotations: map[string]string{ + "elbv2.services.k8s.aws/weight-management": "managed", + }, + expected: false, + }, + { + name: "other annotations present but not weight-management", + annotations: map[string]string{ + "some.other.annotation": "value", + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := &svcapitypes.Rule{} + r.SetAnnotations(tt.annotations) + res := &resource{ko: r} + result := isWeightManagementIgnored(res) + if result != tt.expected { + t.Errorf("isWeightManagementIgnored() = %v, want %v", result, tt.expected) + } + }) + } +} + +func TestMergeLatestWeights(t *testing.T) { + t.Run("merges weights for matching TGs", func(t *testing.T) { + desired := &resource{ko: &svcapitypes.Rule{ + Spec: svcapitypes.RuleSpec{ + Actions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(100)}, + {TargetGroupARN: ptr("arn:aws:tg:green"), Weight: int64Ptr(0)}, + }, + }, + }, + }, + }, + }} + latest := &resource{ko: &svcapitypes.Rule{ + Spec: svcapitypes.RuleSpec{ + Actions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(70)}, + {TargetGroupARN: ptr("arn:aws:tg:green"), Weight: int64Ptr(30)}, + }, + }, + }, + }, + }, + }} + + mergeLatestWeights(desired, latest) + + blueWeight := *desired.ko.Spec.Actions[0].ForwardConfig.TargetGroups[0].Weight + greenWeight := *desired.ko.Spec.Actions[0].ForwardConfig.TargetGroups[1].Weight + + if blueWeight != 70 { + t.Errorf("expected blue weight 70, got %d", blueWeight) + } + if greenWeight != 30 { + t.Errorf("expected green weight 30, got %d", greenWeight) + } + }) + + t.Run("does not change weights for TGs not in latest", func(t *testing.T) { + desired := &resource{ko: &svcapitypes.Rule{ + Spec: svcapitypes.RuleSpec{ + Actions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(100)}, + {TargetGroupARN: ptr("arn:aws:tg:new-green"), Weight: int64Ptr(0)}, + }, + }, + }, + }, + }, + }} + latest := &resource{ko: &svcapitypes.Rule{ + Spec: svcapitypes.RuleSpec{ + Actions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(70)}, + }, + }, + }, + }, + }, + }} + + mergeLatestWeights(desired, latest) + + blueWeight := *desired.ko.Spec.Actions[0].ForwardConfig.TargetGroups[0].Weight + newGreenWeight := *desired.ko.Spec.Actions[0].ForwardConfig.TargetGroups[1].Weight + + if blueWeight != 70 { + t.Errorf("expected blue weight 70 (merged from latest), got %d", blueWeight) + } + if newGreenWeight != 0 { + t.Errorf("expected new-green weight 0 (unchanged, not in latest), got %d", newGreenWeight) + } + }) + + t.Run("handles nil latest gracefully", func(t *testing.T) { + desired := &resource{ko: &svcapitypes.Rule{ + Spec: svcapitypes.RuleSpec{ + Actions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(100)}, + }, + }, + }, + }, + }, + }} + + // Should not panic + mergeLatestWeights(desired, nil) + mergeLatestWeights(nil, nil) + + weight := *desired.ko.Spec.Actions[0].ForwardConfig.TargetGroups[0].Weight + if weight != 100 { + t.Errorf("expected weight unchanged (100), got %d", weight) + } + }) +} + +func TestCustomPreCompareWeightManagement(t *testing.T) { + t.Run("with annotation: copies weights, no delta on weight-only change", func(t *testing.T) { + desired := &resource{ko: &svcapitypes.Rule{ + Spec: svcapitypes.RuleSpec{ + Actions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(100)}, + {TargetGroupARN: ptr("arn:aws:tg:green"), Weight: int64Ptr(0)}, + }, + }, + }, + }, + Conditions: []*svcapitypes.RuleCondition{}, + }, + }} + desired.ko.SetAnnotations(map[string]string{ + "elbv2.services.k8s.aws/weight-management": "ignore", + }) + + latest := &resource{ko: &svcapitypes.Rule{ + Spec: svcapitypes.RuleSpec{ + Actions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(70)}, + {TargetGroupARN: ptr("arn:aws:tg:green"), Weight: int64Ptr(30)}, + }, + }, + }, + }, + Conditions: []*svcapitypes.RuleCondition{}, + }, + }} + + delta := newResourceDelta(desired, latest) + + if delta.DifferentAt("Spec.Actions") { + t.Error("expected no delta on Spec.Actions when weight-management is ignored") + } + }) + + t.Run("without annotation: detects weight differences as delta", func(t *testing.T) { + desired := &resource{ko: &svcapitypes.Rule{ + Spec: svcapitypes.RuleSpec{ + Actions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(100)}, + {TargetGroupARN: ptr("arn:aws:tg:green"), Weight: int64Ptr(0)}, + }, + }, + }, + }, + Conditions: []*svcapitypes.RuleCondition{}, + }, + }} + + latest := &resource{ko: &svcapitypes.Rule{ + Spec: svcapitypes.RuleSpec{ + Actions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(70)}, + {TargetGroupARN: ptr("arn:aws:tg:green"), Weight: int64Ptr(30)}, + }, + }, + }, + }, + Conditions: []*svcapitypes.RuleCondition{}, + }, + }} + + delta := newResourceDelta(desired, latest) + + if !delta.DifferentAt("Spec.Actions") { + t.Error("expected delta on Spec.Actions when weight-management is NOT set") + } + }) + + t.Run("with annotation: adding a new TG still creates delta", func(t *testing.T) { + desired := &resource{ko: &svcapitypes.Rule{ + Spec: svcapitypes.RuleSpec{ + Actions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(100)}, + {TargetGroupARN: ptr("arn:aws:tg:green"), Weight: int64Ptr(0)}, + }, + }, + }, + }, + Conditions: []*svcapitypes.RuleCondition{}, + }, + }} + desired.ko.SetAnnotations(map[string]string{ + "elbv2.services.k8s.aws/weight-management": "ignore", + }) + + latest := &resource{ko: &svcapitypes.Rule{ + Spec: svcapitypes.RuleSpec{ + Actions: []*svcapitypes.Action{ + { + Type: ptr("forward"), + ForwardConfig: &svcapitypes.ForwardActionConfig{ + TargetGroups: []*svcapitypes.TargetGroupTuple{ + {TargetGroupARN: ptr("arn:aws:tg:blue"), Weight: int64Ptr(70)}, + }, + }, + }, + }, + Conditions: []*svcapitypes.RuleCondition{}, + }, + }} + + delta := newResourceDelta(desired, latest) + + if !delta.DifferentAt("Spec.Actions") { + t.Error("expected delta when desired has an extra TG (even with weight-management ignored)") + } + }) +} From 5bfc39c977776404de8457b53a93214abb4f237d Mon Sep 17 00:00:00 2001 From: jessemeng Date: Fri, 12 Jun 2026 11:05:58 +0800 Subject: [PATCH 4/4] fix: regenerate ack-generate-metadata.yaml to match generator.yaml checksum The generator.yaml was modified but the metadata file wasn't regenerated, causing the CI verify-code-gen check to fail on file_checksum mismatch. Co-Authored-By: Claude Opus 4.8 --- apis/v1alpha1/ack-generate-metadata.yaml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/apis/v1alpha1/ack-generate-metadata.yaml b/apis/v1alpha1/ack-generate-metadata.yaml index c8ebc4b..0dc8db9 100755 --- a/apis/v1alpha1/ack-generate-metadata.yaml +++ b/apis/v1alpha1/ack-generate-metadata.yaml @@ -1,13 +1,13 @@ ack_generate_info: - build_date: "2026-05-30T22:41:57Z" - build_hash: a307e8ebd9503616baf5915c744a30dc3aa227c5 - go_version: go1.26.3 - version: v0.59.1-4-ga307e8e + build_date: "2026-06-12T02:08:23Z" + build_hash: 2970ca9b3789515150e27ab80630175a8c77cba4 + go_version: go1.26.4 + version: v0.59.1-7-g2970ca9 api_directory_checksum: 060554dd6962e2466013922cf96fb4cf92a23706 api_version: v1alpha1 aws_sdk_go_version: v1.32.6 generator_config_info: - file_checksum: ce1168f649f03d9652bc8e82f8322d6dd2d909f0 + file_checksum: bac271c7c08e6fc537c9665c20dbf2e37270ddd3 original_file_name: generator.yaml last_modification: reason: API generation